1
Fork 0

Auto merge of #80296 - wesleywiser:revert_missing_fragment_specifier_hard_error, r=Mark-Simulacrum

Revert missing fragment specifier hard error

Closes #76605

Reopens #40107

r? `@Mark-Simulacrum`
This commit is contained in:
bors 2020-12-25 14:09:08 +00:00
commit 1832bdd7de
16 changed files with 135 additions and 35 deletions

View file

@ -84,7 +84,7 @@ enum TokenTree {
/// e.g., `$var` /// e.g., `$var`
MetaVar(Span, Ident), MetaVar(Span, Ident),
/// e.g., `$var:expr`. This is only used in the left hand side of MBE macros. /// e.g., `$var:expr`. This is only used in the left hand side of MBE macros.
MetaVarDecl(Span, Ident /* name to bind */, NonterminalKind), MetaVarDecl(Span, Ident /* name to bind */, Option<NonterminalKind>),
} }
impl TokenTree { impl TokenTree {

View file

@ -378,6 +378,11 @@ fn nameize<I: Iterator<Item = NamedMatch>>(
n_rec(sess, next_m, res.by_ref(), ret_val)?; n_rec(sess, next_m, res.by_ref(), ret_val)?;
} }
} }
TokenTree::MetaVarDecl(span, _, None) => {
if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
return Err((span, "missing fragment specifier".to_string()));
}
}
TokenTree::MetaVarDecl(sp, bind_name, _) => match ret_val TokenTree::MetaVarDecl(sp, bind_name, _) => match ret_val
.entry(MacroRulesNormalizedIdent::new(bind_name)) .entry(MacroRulesNormalizedIdent::new(bind_name))
{ {
@ -446,6 +451,7 @@ fn or_pat_mode(edition: Edition) -> OrPatNonterminalMode {
/// ///
/// A `ParseResult`. Note that matches are kept track of through the items generated. /// A `ParseResult`. Note that matches are kept track of through the items generated.
fn inner_parse_loop<'root, 'tt>( fn inner_parse_loop<'root, 'tt>(
sess: &ParseSess,
cur_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>, cur_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
next_items: &mut Vec<MatcherPosHandle<'root, 'tt>>, next_items: &mut Vec<MatcherPosHandle<'root, 'tt>>,
eof_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>, eof_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
@ -563,9 +569,16 @@ fn inner_parse_loop<'root, 'tt>(
}))); })));
} }
// We need to match a metavar (but the identifier is invalid)... this is an error
TokenTree::MetaVarDecl(span, _, None) => {
if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
return Error(span, "missing fragment specifier".to_string());
}
}
// We need to match a metavar with a valid ident... call out to the black-box // We need to match a metavar with a valid ident... call out to the black-box
// parser by adding an item to `bb_items`. // parser by adding an item to `bb_items`.
TokenTree::MetaVarDecl(span, _, kind) => { TokenTree::MetaVarDecl(span, _, Some(kind)) => {
// Built-in nonterminals never start with these tokens, so we can eliminate // Built-in nonterminals never start with these tokens, so we can eliminate
// them from consideration. // them from consideration.
// //
@ -640,6 +653,7 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na
// parsing from the black-box parser done. The result is that `next_items` will contain a // parsing from the black-box parser done. The result is that `next_items` will contain a
// bunch of possible next matcher positions in `next_items`. // bunch of possible next matcher positions in `next_items`.
match inner_parse_loop( match inner_parse_loop(
parser.sess,
&mut cur_items, &mut cur_items,
&mut next_items, &mut next_items,
&mut eof_items, &mut eof_items,
@ -701,7 +715,7 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na
let nts = bb_items let nts = bb_items
.iter() .iter()
.map(|item| match item.top_elts.get_tt(item.idx) { .map(|item| match item.top_elts.get_tt(item.idx) {
TokenTree::MetaVarDecl(_, bind, kind) => format!("{} ('{}')", kind, bind), TokenTree::MetaVarDecl(_, bind, Some(kind)) => format!("{} ('{}')", kind, bind),
_ => panic!(), _ => panic!(),
}) })
.collect::<Vec<String>>() .collect::<Vec<String>>()
@ -731,7 +745,7 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na
assert_eq!(bb_items.len(), 1); assert_eq!(bb_items.len(), 1);
let mut item = bb_items.pop().unwrap(); let mut item = bb_items.pop().unwrap();
if let TokenTree::MetaVarDecl(span, _, kind) = item.top_elts.get_tt(item.idx) { if let TokenTree::MetaVarDecl(span, _, Some(kind)) = item.top_elts.get_tt(item.idx) {
let match_cur = item.match_cur; let match_cur = item.match_cur;
// We use the span of the metavariable declaration to determine any // We use the span of the metavariable declaration to determine any
// edition-specific matching behavior for non-terminals. // edition-specific matching behavior for non-terminals.

View file

@ -401,7 +401,7 @@ pub fn compile_declarative_macro(
let diag = &sess.parse_sess.span_diagnostic; let diag = &sess.parse_sess.span_diagnostic;
let lhs_nm = Ident::new(sym::lhs, def.span); let lhs_nm = Ident::new(sym::lhs, def.span);
let rhs_nm = Ident::new(sym::rhs, def.span); let rhs_nm = Ident::new(sym::rhs, def.span);
let tt_spec = NonterminalKind::TT; let tt_spec = Some(NonterminalKind::TT);
// Parse the macro_rules! invocation // Parse the macro_rules! invocation
let (macro_rules, body) = match &def.kind { let (macro_rules, body) = match &def.kind {
@ -578,7 +578,7 @@ fn check_lhs_no_empty_seq(sess: &ParseSess, tts: &[mbe::TokenTree]) -> bool {
TokenTree::Sequence(span, ref seq) => { TokenTree::Sequence(span, ref seq) => {
if seq.separator.is_none() if seq.separator.is_none()
&& seq.tts.iter().all(|seq_tt| match *seq_tt { && seq.tts.iter().all(|seq_tt| match *seq_tt {
TokenTree::MetaVarDecl(_, _, NonterminalKind::Vis) => true, TokenTree::MetaVarDecl(_, _, Some(NonterminalKind::Vis)) => true,
TokenTree::Sequence(_, ref sub_seq) => { TokenTree::Sequence(_, ref sub_seq) => {
sub_seq.kleene.op == mbe::KleeneOp::ZeroOrMore sub_seq.kleene.op == mbe::KleeneOp::ZeroOrMore
|| sub_seq.kleene.op == mbe::KleeneOp::ZeroOrOne || sub_seq.kleene.op == mbe::KleeneOp::ZeroOrOne
@ -961,7 +961,7 @@ fn check_matcher_core(
// Now `last` holds the complete set of NT tokens that could // Now `last` holds the complete set of NT tokens that could
// end the sequence before SUFFIX. Check that every one works with `suffix`. // end the sequence before SUFFIX. Check that every one works with `suffix`.
for token in &last.tokens { for token in &last.tokens {
if let TokenTree::MetaVarDecl(_, name, kind) = *token { if let TokenTree::MetaVarDecl(_, name, Some(kind)) = *token {
for next_token in &suffix_first.tokens { for next_token in &suffix_first.tokens {
match is_in_follow(next_token, kind) { match is_in_follow(next_token, kind) {
IsInFollow::Yes => {} IsInFollow::Yes => {}
@ -1019,7 +1019,7 @@ fn check_matcher_core(
} }
fn token_can_be_followed_by_any(tok: &mbe::TokenTree) -> bool { fn token_can_be_followed_by_any(tok: &mbe::TokenTree) -> bool {
if let mbe::TokenTree::MetaVarDecl(_, _, kind) = *tok { if let mbe::TokenTree::MetaVarDecl(_, _, Some(kind)) = *tok {
frag_can_be_followed_by_any(kind) frag_can_be_followed_by_any(kind)
} else { } else {
// (Non NT's can always be followed by anything in matchers.) // (Non NT's can always be followed by anything in matchers.)
@ -1123,7 +1123,7 @@ fn is_in_follow(tok: &mbe::TokenTree, kind: NonterminalKind) -> IsInFollow {
} }
_ => IsInFollow::No(TOKENS), _ => IsInFollow::No(TOKENS),
}, },
TokenTree::MetaVarDecl(_, _, NonterminalKind::Block) => IsInFollow::Yes, TokenTree::MetaVarDecl(_, _, Some(NonterminalKind::Block)) => IsInFollow::Yes,
_ => IsInFollow::No(TOKENS), _ => IsInFollow::No(TOKENS),
} }
} }
@ -1158,7 +1158,7 @@ fn is_in_follow(tok: &mbe::TokenTree, kind: NonterminalKind) -> IsInFollow {
TokenTree::MetaVarDecl( TokenTree::MetaVarDecl(
_, _,
_, _,
NonterminalKind::Ident | NonterminalKind::Ty | NonterminalKind::Path, Some(NonterminalKind::Ident | NonterminalKind::Ty | NonterminalKind::Path),
) => IsInFollow::Yes, ) => IsInFollow::Yes,
_ => IsInFollow::No(TOKENS), _ => IsInFollow::No(TOKENS),
} }
@ -1171,7 +1171,8 @@ fn quoted_tt_to_string(tt: &mbe::TokenTree) -> String {
match *tt { match *tt {
mbe::TokenTree::Token(ref token) => pprust::token_to_string(&token), mbe::TokenTree::Token(ref token) => pprust::token_to_string(&token),
mbe::TokenTree::MetaVar(_, name) => format!("${}", name), mbe::TokenTree::MetaVar(_, name) => format!("${}", name),
mbe::TokenTree::MetaVarDecl(_, name, kind) => format!("${}:{}", name, kind), mbe::TokenTree::MetaVarDecl(_, name, Some(kind)) => format!("${}:{}", name, kind),
mbe::TokenTree::MetaVarDecl(_, name, None) => format!("${}:", name),
_ => panic!( _ => panic!(
"{}", "{}",
"unexpected mbe::TokenTree::{Sequence or Delimited} \ "unexpected mbe::TokenTree::{Sequence or Delimited} \

View file

@ -3,7 +3,7 @@ use crate::mbe::{Delimited, KleeneOp, KleeneToken, SequenceRepetition, TokenTree
use rustc_ast::token::{self, Token}; use rustc_ast::token::{self, Token};
use rustc_ast::tokenstream; use rustc_ast::tokenstream;
use rustc_ast::NodeId; use rustc_ast::{NodeId, DUMMY_NODE_ID};
use rustc_ast_pretty::pprust; use rustc_ast_pretty::pprust;
use rustc_session::parse::ParseSess; use rustc_session::parse::ParseSess;
use rustc_span::symbol::{kw, Ident}; use rustc_span::symbol::{kw, Ident};
@ -73,7 +73,7 @@ pub(super) fn parse(
.emit(); .emit();
token::NonterminalKind::Ident token::NonterminalKind::Ident
}); });
result.push(TokenTree::MetaVarDecl(span, ident, kind)); result.push(TokenTree::MetaVarDecl(span, ident, Some(kind)));
continue; continue;
} }
_ => token.span, _ => token.span,
@ -83,8 +83,11 @@ pub(super) fn parse(
} }
tree => tree.as_ref().map(tokenstream::TokenTree::span).unwrap_or(start_sp), tree => tree.as_ref().map(tokenstream::TokenTree::span).unwrap_or(start_sp),
}; };
sess.span_diagnostic.struct_span_err(span, "missing fragment specifier").emit(); if node_id != DUMMY_NODE_ID {
continue; // Macros loaded from other crates have dummy node ids.
sess.missing_fragment_specifiers.borrow_mut().insert(span, node_id);
}
result.push(TokenTree::MetaVarDecl(span, ident, None));
} }
// Not a metavar or no matchers allowed, so just return the tree // Not a metavar or no matchers allowed, so just return the tree

View file

@ -29,6 +29,7 @@ use rustc_passes::{self, hir_stats, layout_test};
use rustc_plugin_impl as plugin; use rustc_plugin_impl as plugin;
use rustc_resolve::{Resolver, ResolverArenas}; use rustc_resolve::{Resolver, ResolverArenas};
use rustc_session::config::{CrateType, Input, OutputFilenames, OutputType, PpMode, PpSourceMode}; use rustc_session::config::{CrateType, Input, OutputFilenames, OutputType, PpMode, PpSourceMode};
use rustc_session::lint;
use rustc_session::output::{filename_for_input, filename_for_metadata}; use rustc_session::output::{filename_for_input, filename_for_metadata};
use rustc_session::search_paths::PathKind; use rustc_session::search_paths::PathKind;
use rustc_session::Session; use rustc_session::Session;
@ -306,11 +307,27 @@ fn configure_and_expand_inner<'a>(
ecx.check_unused_macros(); ecx.check_unused_macros();
}); });
let mut missing_fragment_specifiers: Vec<_> = ecx
.sess
.parse_sess
.missing_fragment_specifiers
.borrow()
.iter()
.map(|(span, node_id)| (*span, *node_id))
.collect();
missing_fragment_specifiers.sort_unstable_by_key(|(span, _)| *span);
let recursion_limit_hit = ecx.reduced_recursion_limit.is_some();
for (span, node_id) in missing_fragment_specifiers {
let lint = lint::builtin::MISSING_FRAGMENT_SPECIFIER;
let msg = "missing fragment specifier";
resolver.lint_buffer().buffer_lint(lint, node_id, span, msg);
}
if cfg!(windows) { if cfg!(windows) {
env::set_var("PATH", &old_path); env::set_var("PATH", &old_path);
} }
let recursion_limit_hit = ecx.reduced_recursion_limit.is_some();
if recursion_limit_hit { if recursion_limit_hit {
// If we hit a recursion limit, exit early to avoid later passes getting overwhelmed // If we hit a recursion limit, exit early to avoid later passes getting overwhelmed
// with a large AST // with a large AST

View file

@ -1227,6 +1227,50 @@ declare_lint! {
}; };
} }
declare_lint! {
/// The `missing_fragment_specifier` lint is issued when an unused pattern in a
/// `macro_rules!` macro definition has a meta-variable (e.g. `$e`) that is not
/// followed by a fragment specifier (e.g. `:expr`).
///
/// This warning can always be fixed by removing the unused pattern in the
/// `macro_rules!` macro definition.
///
/// ### Example
///
/// ```rust,compile_fail
/// macro_rules! foo {
/// () => {};
/// ($name) => { };
/// }
///
/// fn main() {
/// foo!();
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// To fix this, remove the unused pattern from the `macro_rules!` macro definition:
///
/// ```rust
/// macro_rules! foo {
/// () => {};
/// }
/// fn main() {
/// foo!();
/// }
/// ```
pub MISSING_FRAGMENT_SPECIFIER,
Deny,
"detects missing fragment specifiers in unused `macro_rules!` patterns",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #40107 <https://github.com/rust-lang/rust/issues/40107>",
edition: None,
};
}
declare_lint! { declare_lint! {
/// The `late_bound_lifetime_arguments` lint detects generic lifetime /// The `late_bound_lifetime_arguments` lint detects generic lifetime
/// arguments in path segments with late bound lifetime parameters. /// arguments in path segments with late bound lifetime parameters.
@ -2827,6 +2871,7 @@ declare_lint_pass! {
CONST_ITEM_MUTATION, CONST_ITEM_MUTATION,
SAFE_PACKED_BORROWS, SAFE_PACKED_BORROWS,
PATTERNS_IN_FNS_WITHOUT_BODY, PATTERNS_IN_FNS_WITHOUT_BODY,
MISSING_FRAGMENT_SPECIFIER,
LATE_BOUND_LIFETIME_ARGUMENTS, LATE_BOUND_LIFETIME_ARGUMENTS,
ORDER_DEPENDENT_TRAIT_OBJECTS, ORDER_DEPENDENT_TRAIT_OBJECTS,
COHERENCE_LEAK_CHECK, COHERENCE_LEAK_CHECK,

View file

@ -119,6 +119,7 @@ pub struct ParseSess {
pub unstable_features: UnstableFeatures, pub unstable_features: UnstableFeatures,
pub config: CrateConfig, pub config: CrateConfig,
pub edition: Edition, pub edition: Edition,
pub missing_fragment_specifiers: Lock<FxHashMap<Span, NodeId>>,
/// Places where raw identifiers were used. This is used for feature-gating raw identifiers. /// Places where raw identifiers were used. This is used for feature-gating raw identifiers.
pub raw_identifier_spans: Lock<Vec<Span>>, pub raw_identifier_spans: Lock<Vec<Span>>,
/// Used to determine and report recursive module inclusions. /// Used to determine and report recursive module inclusions.
@ -152,6 +153,7 @@ impl ParseSess {
unstable_features: UnstableFeatures::from_environment(None), unstable_features: UnstableFeatures::from_environment(None),
config: FxHashSet::default(), config: FxHashSet::default(),
edition: ExpnId::root().expn_data().edition, edition: ExpnId::root().expn_data().edition,
missing_fragment_specifiers: Default::default(),
raw_identifier_spans: Lock::new(Vec::new()), raw_identifier_spans: Lock::new(Vec::new()),
included_mod_stack: Lock::new(vec![]), included_mod_stack: Lock::new(vec![]),
source_map, source_map,

View file

@ -1,3 +0,0 @@
# Deny-by-default lints
This file is auto-generated by the lint-docs script.

View file

@ -5,6 +5,10 @@ macro_rules! foo {
( $($i:ident)* ) => { $($i)+ }; //~ WARN meta-variable repeats with different Kleene operator ( $($i:ident)* ) => { $($i)+ }; //~ WARN meta-variable repeats with different Kleene operator
} }
#[warn(missing_fragment_specifier)]
macro_rules! m { ($i) => {} } //~ WARN missing fragment specifier
//~| WARN this was previously accepted
#[warn(soft_unstable)] #[warn(soft_unstable)]
mod benches { mod benches {
#[bench] //~ WARN use of unstable library feature 'test' #[bench] //~ WARN use of unstable library feature 'test'

View file

@ -12,14 +12,28 @@ note: the lint level is defined here
LL | #[warn(meta_variable_misuse)] LL | #[warn(meta_variable_misuse)]
| ^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^
warning: missing fragment specifier
--> $DIR/expansion-time.rs:9:19
|
LL | macro_rules! m { ($i) => {} }
| ^^
|
note: the lint level is defined here
--> $DIR/expansion-time.rs:8:8
|
LL | #[warn(missing_fragment_specifier)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
warning: use of unstable library feature 'test': `bench` is a part of custom test frameworks which are unstable warning: use of unstable library feature 'test': `bench` is a part of custom test frameworks which are unstable
--> $DIR/expansion-time.rs:10:7 --> $DIR/expansion-time.rs:14:7
| |
LL | #[bench] LL | #[bench]
| ^^^^^ | ^^^^^
| |
note: the lint level is defined here note: the lint level is defined here
--> $DIR/expansion-time.rs:8:8 --> $DIR/expansion-time.rs:12:8
| |
LL | #[warn(soft_unstable)] LL | #[warn(soft_unstable)]
| ^^^^^^^^^^^^^ | ^^^^^^^^^^^^^
@ -33,10 +47,10 @@ LL | 2
| ^ | ^
| |
note: the lint level is defined here note: the lint level is defined here
--> $DIR/expansion-time.rs:25:8 --> $DIR/expansion-time.rs:29:8
| |
LL | #[warn(incomplete_include)] LL | #[warn(incomplete_include)]
| ^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^
warning: 3 warnings emitted warning: 4 warnings emitted

View file

@ -2,5 +2,6 @@
macro_rules! m { ($i) => {} } macro_rules! m { ($i) => {} }
//~^ ERROR missing fragment specifier //~^ ERROR missing fragment specifier
//~| WARN previously accepted
fn main() {} fn main() {}

View file

@ -3,6 +3,10 @@ error: missing fragment specifier
| |
LL | macro_rules! m { ($i) => {} } LL | macro_rules! m { ($i) => {} }
| ^^ | ^^
|
= note: `#[deny(missing_fragment_specifier)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
error: aborting due to previous error error: aborting due to previous error

View file

@ -2,6 +2,7 @@ macro_rules! test {
($a, $b) => { ($a, $b) => {
//~^ ERROR missing fragment //~^ ERROR missing fragment
//~| ERROR missing fragment //~| ERROR missing fragment
//~| WARN this was previously accepted
() ()
}; };
} }

View file

@ -9,6 +9,10 @@ error: missing fragment specifier
| |
LL | ($a, $b) => { LL | ($a, $b) => {
| ^^ | ^^
|
= note: `#[deny(missing_fragment_specifier)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
error: aborting due to 2 previous errors error: aborting due to 2 previous errors

View file

@ -2,7 +2,6 @@ macro_rules! foo {
{ $+ } => { //~ ERROR expected identifier, found `+` { $+ } => { //~ ERROR expected identifier, found `+`
//~^ ERROR missing fragment specifier //~^ ERROR missing fragment specifier
$(x)(y) //~ ERROR expected one of: `*`, `+`, or `?` $(x)(y) //~ ERROR expected one of: `*`, `+`, or `?`
//~^ ERROR attempted to repeat an expression containing no syntax variables
} }
} }

View file

@ -4,23 +4,17 @@ error: expected identifier, found `+`
LL | { $+ } => { LL | { $+ } => {
| ^ | ^
error: missing fragment specifier
--> $DIR/issue-33569.rs:2:8
|
LL | { $+ } => {
| ^
error: expected one of: `*`, `+`, or `?` error: expected one of: `*`, `+`, or `?`
--> $DIR/issue-33569.rs:4:13 --> $DIR/issue-33569.rs:4:13
| |
LL | $(x)(y) LL | $(x)(y)
| ^^^ | ^^^
error: attempted to repeat an expression containing no syntax variables matched as repeating at this depth error: missing fragment specifier
--> $DIR/issue-33569.rs:4:10 --> $DIR/issue-33569.rs:2:8
| |
LL | $(x)(y) LL | { $+ } => {
| ^^^ | ^
error: aborting due to 4 previous errors error: aborting due to 3 previous errors