Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert missing fragment specifier hard error #80296

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/mbe.rs
Original file line number Diff line number Diff line change
@@ -84,7 +84,7 @@ enum TokenTree {
/// e.g., `$var`
MetaVar(Span, Ident),
/// 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 {
20 changes: 17 additions & 3 deletions compiler/rustc_expand/src/mbe/macro_parser.rs
Original file line number Diff line number Diff line change
@@ -378,6 +378,11 @@ fn nameize<I: Iterator<Item = NamedMatch>>(
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
.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.
fn inner_parse_loop<'root, 'tt>(
sess: &ParseSess,
cur_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
next_items: &mut Vec<MatcherPosHandle<'root, 'tt>>,
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
// 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
// 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
// bunch of possible next matcher positions in `next_items`.
match inner_parse_loop(
parser.sess,
&mut cur_items,
&mut next_items,
&mut eof_items,
@@ -701,7 +715,7 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na
let nts = bb_items
.iter()
.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!(),
})
.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);

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;
// We use the span of the metavariable declaration to determine any
// edition-specific matching behavior for non-terminals.
15 changes: 8 additions & 7 deletions compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
@@ -401,7 +401,7 @@ pub fn compile_declarative_macro(
let diag = &sess.parse_sess.span_diagnostic;
let lhs_nm = Ident::new(sym::lhs, 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
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) => {
if seq.separator.is_none()
&& 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) => {
sub_seq.kleene.op == mbe::KleeneOp::ZeroOrMore
|| 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
// end the sequence before SUFFIX. Check that every one works with `suffix`.
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 {
match is_in_follow(next_token, kind) {
IsInFollow::Yes => {}
@@ -1019,7 +1019,7 @@ fn check_matcher_core(
}

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)
} else {
// (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),
},
TokenTree::MetaVarDecl(_, _, NonterminalKind::Block) => IsInFollow::Yes,
TokenTree::MetaVarDecl(_, _, Some(NonterminalKind::Block)) => IsInFollow::Yes,
_ => IsInFollow::No(TOKENS),
}
}
@@ -1158,7 +1158,7 @@ fn is_in_follow(tok: &mbe::TokenTree, kind: NonterminalKind) -> IsInFollow {
TokenTree::MetaVarDecl(
_,
_,
NonterminalKind::Ident | NonterminalKind::Ty | NonterminalKind::Path,
Some(NonterminalKind::Ident | NonterminalKind::Ty | NonterminalKind::Path),
) => IsInFollow::Yes,
_ => IsInFollow::No(TOKENS),
}
@@ -1171,7 +1171,8 @@ fn quoted_tt_to_string(tt: &mbe::TokenTree) -> String {
match *tt {
mbe::TokenTree::Token(ref token) => pprust::token_to_string(&token),
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!(
"{}",
"unexpected mbe::TokenTree::{Sequence or Delimited} \
11 changes: 7 additions & 4 deletions compiler/rustc_expand/src/mbe/quoted.rs
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@ use crate::mbe::{Delimited, KleeneOp, KleeneToken, SequenceRepetition, TokenTree

use rustc_ast::token::{self, Token};
use rustc_ast::tokenstream;
use rustc_ast::NodeId;
use rustc_ast::{NodeId, DUMMY_NODE_ID};
use rustc_ast_pretty::pprust;
use rustc_session::parse::ParseSess;
use rustc_span::symbol::{kw, Ident};
@@ -73,7 +73,7 @@ pub(super) fn parse(
.emit();
token::NonterminalKind::Ident
});
result.push(TokenTree::MetaVarDecl(span, ident, kind));
result.push(TokenTree::MetaVarDecl(span, ident, Some(kind)));
continue;
}
_ => token.span,
@@ -83,8 +83,11 @@ pub(super) fn parse(
}
tree => tree.as_ref().map(tokenstream::TokenTree::span).unwrap_or(start_sp),
};
sess.span_diagnostic.struct_span_err(span, "missing fragment specifier").emit();
continue;
if node_id != DUMMY_NODE_ID {
// 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
19 changes: 18 additions & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
@@ -29,6 +29,7 @@ use rustc_passes::{self, hir_stats, layout_test};
use rustc_plugin_impl as plugin;
use rustc_resolve::{Resolver, ResolverArenas};
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::search_paths::PathKind;
use rustc_session::Session;
@@ -306,11 +307,27 @@ fn configure_and_expand_inner<'a>(
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) {
env::set_var("PATH", &old_path);
}

let recursion_limit_hit = ecx.reduced_recursion_limit.is_some();
if recursion_limit_hit {
// If we hit a recursion limit, exit early to avoid later passes getting overwhelmed
// with a large AST
45 changes: 45 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
@@ -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! {
/// The `late_bound_lifetime_arguments` lint detects generic lifetime
/// arguments in path segments with late bound lifetime parameters.
@@ -2827,6 +2871,7 @@ declare_lint_pass! {
CONST_ITEM_MUTATION,
SAFE_PACKED_BORROWS,
PATTERNS_IN_FNS_WITHOUT_BODY,
MISSING_FRAGMENT_SPECIFIER,
LATE_BOUND_LIFETIME_ARGUMENTS,
ORDER_DEPENDENT_TRAIT_OBJECTS,
COHERENCE_LEAK_CHECK,
2 changes: 2 additions & 0 deletions compiler/rustc_session/src/parse.rs
Original file line number Diff line number Diff line change
@@ -119,6 +119,7 @@ pub struct ParseSess {
pub unstable_features: UnstableFeatures,
pub config: CrateConfig,
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.
pub raw_identifier_spans: Lock<Vec<Span>>,
/// Used to determine and report recursive module inclusions.
@@ -152,6 +153,7 @@ impl ParseSess {
unstable_features: UnstableFeatures::from_environment(None),
config: FxHashSet::default(),
edition: ExpnId::root().expn_data().edition,
missing_fragment_specifiers: Default::default(),
raw_identifier_spans: Lock::new(Vec::new()),
included_mod_stack: Lock::new(vec![]),
source_map,
3 changes: 0 additions & 3 deletions src/doc/rustc/src/lints/listing/deny-by-default.md

This file was deleted.

4 changes: 4 additions & 0 deletions src/test/ui/lint/expansion-time.rs
Original file line number Diff line number Diff line change
@@ -5,6 +5,10 @@ macro_rules! foo {
( $($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)]
mod benches {
#[bench] //~ WARN use of unstable library feature 'test'
22 changes: 18 additions & 4 deletions src/test/ui/lint/expansion-time.stderr
Original file line number Diff line number Diff line change
@@ -12,14 +12,28 @@ note: the lint level is defined here
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
--> $DIR/expansion-time.rs:10:7
--> $DIR/expansion-time.rs:14:7
|
LL | #[bench]
| ^^^^^
|
note: the lint level is defined here
--> $DIR/expansion-time.rs:8:8
--> $DIR/expansion-time.rs:12:8
|
LL | #[warn(soft_unstable)]
| ^^^^^^^^^^^^^
@@ -33,10 +47,10 @@ LL | 2
| ^
|
note: the lint level is defined here
--> $DIR/expansion-time.rs:25:8
--> $DIR/expansion-time.rs:29:8
|
LL | #[warn(incomplete_include)]
| ^^^^^^^^^^^^^^^^^^

warning: 3 warnings emitted
warning: 4 warnings emitted

1 change: 1 addition & 0 deletions src/test/ui/macros/issue-39404.rs
Original file line number Diff line number Diff line change
@@ -2,5 +2,6 @@

macro_rules! m { ($i) => {} }
//~^ ERROR missing fragment specifier
//~| WARN previously accepted

fn main() {}
4 changes: 4 additions & 0 deletions src/test/ui/macros/issue-39404.stderr
Original file line number Diff line number Diff line change
@@ -3,6 +3,10 @@ error: missing fragment specifier
|
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

1 change: 1 addition & 0 deletions src/test/ui/macros/macro-match-nonterminal.rs
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@ macro_rules! test {
($a, $b) => {
//~^ ERROR missing fragment
//~| ERROR missing fragment
//~| WARN this was previously accepted
()
};
}
4 changes: 4 additions & 0 deletions src/test/ui/macros/macro-match-nonterminal.stderr
Original file line number Diff line number Diff line change
@@ -9,6 +9,10 @@ error: missing fragment specifier
|
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

1 change: 0 additions & 1 deletion src/test/ui/parser/macro/issue-33569.rs
Original file line number Diff line number Diff line change
@@ -2,7 +2,6 @@ macro_rules! foo {
{ $+ } => { //~ ERROR expected identifier, found `+`
//~^ ERROR missing fragment specifier
$(x)(y) //~ ERROR expected one of: `*`, `+`, or `?`
//~^ ERROR attempted to repeat an expression containing no syntax variables
}
}

16 changes: 5 additions & 11 deletions src/test/ui/parser/macro/issue-33569.stderr
Original file line number Diff line number Diff line change
@@ -4,23 +4,17 @@ error: expected identifier, found `+`
LL | { $+ } => {
| ^

error: missing fragment specifier
--> $DIR/issue-33569.rs:2:8
|
LL | { $+ } => {
| ^

error: expected one of: `*`, `+`, or `?`
--> $DIR/issue-33569.rs:4:13
|
LL | $(x)(y)
| ^^^

error: attempted to repeat an expression containing no syntax variables matched as repeating at this depth
--> $DIR/issue-33569.rs:4:10
error: missing fragment specifier
--> $DIR/issue-33569.rs:2:8
|
LL | $(x)(y)
| ^^^
LL | { $+ } => {
| ^

error: aborting due to 4 previous errors
error: aborting due to 3 previous errors