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

Promote missing_fragment_specifier to hard error #75516

Merged
merged 4 commits into from Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 0 additions & 9 deletions src/doc/rustc/src/lints/listing/deny-by-default.md
Expand Up @@ -45,15 +45,6 @@ error: defaults for type parameters are only allowed in `struct`, `enum`, `type`
= note: for more information, see issue #36887 <https://github.com/rust-lang/rust/issues/36887>
```

## missing-fragment-specifier

The missing_fragment_specifier warning 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.

## mutable-transmutes

This lint catches transmuting from `&T` to `&mut T` because it is undefined
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_expand/mbe.rs
Expand Up @@ -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 */, Option<NonterminalKind>),
MetaVarDecl(Span, Ident /* name to bind */, NonterminalKind),
}

impl TokenTree {
Expand Down
20 changes: 3 additions & 17 deletions src/librustc_expand/mbe/macro_parser.rs
Expand Up @@ -378,11 +378,6 @@ 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))
{
Expand Down Expand Up @@ -442,7 +437,6 @@ fn token_name_eq(t1: &Token, t2: &Token) -> bool {
///
/// 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]>,
Expand Down Expand Up @@ -560,16 +554,9 @@ 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(_, _, Some(kind)) => {
TokenTree::MetaVarDecl(_, _, kind) => {
// Built-in nonterminals never start with these tokens,
// so we can eliminate them from consideration.
if Parser::nonterminal_may_begin_with(kind, token) {
Expand Down Expand Up @@ -640,7 +627,6 @@ 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,
Expand Down Expand Up @@ -702,7 +688,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, Some(kind)) => format!("{} ('{}')", kind, bind),
TokenTree::MetaVarDecl(_, bind, kind) => format!("{} ('{}')", kind, bind),
_ => panic!(),
})
.collect::<Vec<String>>()
Expand Down Expand Up @@ -732,7 +718,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, _, Some(kind)) = item.top_elts.get_tt(item.idx) {
if let TokenTree::MetaVarDecl(span, _, kind) = item.top_elts.get_tt(item.idx) {
let match_cur = item.match_cur;
let nt = match parser.to_mut().parse_nonterminal(kind) {
Err(mut err) => {
Expand Down
15 changes: 7 additions & 8 deletions src/librustc_expand/mbe/macro_rules.rs
Expand Up @@ -400,7 +400,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 = Some(NonterminalKind::TT);
let tt_spec = NonterminalKind::TT;

// Parse the macro_rules! invocation
let (macro_rules, body) = match &def.kind {
Expand Down Expand Up @@ -577,7 +577,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(_, _, Some(NonterminalKind::Vis)) => true,
TokenTree::MetaVarDecl(_, _, NonterminalKind::Vis) => true,
TokenTree::Sequence(_, ref sub_seq) => {
sub_seq.kleene.op == mbe::KleeneOp::ZeroOrMore
|| sub_seq.kleene.op == mbe::KleeneOp::ZeroOrOne
Expand Down Expand Up @@ -960,7 +960,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, Some(kind)) = *token {
if let TokenTree::MetaVarDecl(_, name, kind) = *token {
for next_token in &suffix_first.tokens {
match is_in_follow(next_token, kind) {
IsInFollow::Yes => {}
Expand Down Expand Up @@ -1018,7 +1018,7 @@ fn check_matcher_core(
}

fn token_can_be_followed_by_any(tok: &mbe::TokenTree) -> bool {
if let mbe::TokenTree::MetaVarDecl(_, _, Some(kind)) = *tok {
if let mbe::TokenTree::MetaVarDecl(_, _, kind) = *tok {
frag_can_be_followed_by_any(kind)
} else {
// (Non NT's can always be followed by anything in matchers.)
Expand Down Expand Up @@ -1123,7 +1123,7 @@ fn is_in_follow(tok: &mbe::TokenTree, kind: NonterminalKind) -> IsInFollow {
}
_ => IsInFollow::No(TOKENS),
},
TokenTree::MetaVarDecl(_, _, Some(NonterminalKind::Block)) => IsInFollow::Yes,
TokenTree::MetaVarDecl(_, _, NonterminalKind::Block) => IsInFollow::Yes,
_ => IsInFollow::No(TOKENS),
}
}
Expand Down Expand Up @@ -1158,7 +1158,7 @@ fn is_in_follow(tok: &mbe::TokenTree, kind: NonterminalKind) -> IsInFollow {
TokenTree::MetaVarDecl(
_,
_,
Some(NonterminalKind::Ident | NonterminalKind::Ty | NonterminalKind::Path),
NonterminalKind::Ident | NonterminalKind::Ty | NonterminalKind::Path,
) => IsInFollow::Yes,
_ => IsInFollow::No(TOKENS),
}
Expand All @@ -1171,8 +1171,7 @@ 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, Some(kind)) => format!("${}:{}", name, kind),
mbe::TokenTree::MetaVarDecl(_, name, None) => format!("${}:", name),
mbe::TokenTree::MetaVarDecl(_, name, kind) => format!("${}:{}", name, kind),
_ => panic!(
"unexpected mbe::TokenTree::{{Sequence or Delimited}} \
in follow set checker"
Expand Down
11 changes: 4 additions & 7 deletions src/librustc_expand/mbe/quoted.rs
Expand Up @@ -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, DUMMY_NODE_ID};
use rustc_ast::NodeId;
use rustc_ast_pretty::pprust;
use rustc_session::parse::ParseSess;
use rustc_span::symbol::{kw, Ident};
Expand Down Expand Up @@ -73,7 +73,7 @@ pub(super) fn parse(
.emit();
token::NonterminalKind::Ident
});
result.push(TokenTree::MetaVarDecl(span, ident, Some(kind)));
result.push(TokenTree::MetaVarDecl(span, ident, kind));
continue;
}
_ => token.span,
Expand All @@ -83,11 +83,8 @@ pub(super) fn parse(
}
tree => tree.as_ref().map(tokenstream::TokenTree::span).unwrap_or(start_sp),
};
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));
sess.span_diagnostic.struct_span_err(span, "missing fragment specifier").emit();
continue;
}

// Not a metavar or no matchers allowed, so just return the tree
Expand Down
19 changes: 1 addition & 18 deletions src/librustc_interface/passes.rs
Expand Up @@ -30,7 +30,6 @@ 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;
Expand Down Expand Up @@ -307,27 +306,11 @@ 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
Expand Down
11 changes: 0 additions & 11 deletions src/librustc_session/lint/builtin.rs
Expand Up @@ -252,16 +252,6 @@ declare_lint! {
};
}

declare_lint! {
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! {
pub LATE_BOUND_LIFETIME_ARGUMENTS,
Warn,
Expand Down Expand Up @@ -584,7 +574,6 @@ declare_lint_pass! {
UNALIGNED_REFERENCES,
SAFE_PACKED_BORROWS,
PATTERNS_IN_FNS_WITHOUT_BODY,
MISSING_FRAGMENT_SPECIFIER,
LATE_BOUND_LIFETIME_ARGUMENTS,
ORDER_DEPENDENT_TRAIT_OBJECTS,
COHERENCE_LEAK_CHECK,
Expand Down
2 changes: 0 additions & 2 deletions src/librustc_session/parse.rs
Expand Up @@ -119,7 +119,6 @@ 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.
Expand Down Expand Up @@ -154,7 +153,6 @@ impl ParseSess {
unstable_features: UnstableFeatures::from_environment(),
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,
Expand Down
12 changes: 0 additions & 12 deletions src/test/ui/issues/issue-39404.stderr

This file was deleted.

4 changes: 0 additions & 4 deletions src/test/ui/lint/expansion-time.rs
Expand Up @@ -5,10 +5,6 @@ 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'
Expand Down
22 changes: 4 additions & 18 deletions src/test/ui/lint/expansion-time.stderr
Expand Up @@ -12,28 +12,14 @@ 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:14:7
--> $DIR/expansion-time.rs:10:7
|
LL | #[bench]
| ^^^^^
|
note: the lint level is defined here
--> $DIR/expansion-time.rs:12:8
--> $DIR/expansion-time.rs:8:8
|
LL | #[warn(soft_unstable)]
| ^^^^^^^^^^^^^
Expand All @@ -47,10 +33,10 @@ LL | 2
| ^
|
note: the lint level is defined here
--> $DIR/expansion-time.rs:19:8
--> $DIR/expansion-time.rs:15:8
|
LL | #[warn(incomplete_include)]
| ^^^^^^^^^^^^^^^^^^

warning: 4 warnings emitted
warning: 3 warnings emitted

Expand Up @@ -2,6 +2,5 @@

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

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/macros/issue-39404.stderr
@@ -0,0 +1,8 @@
error: missing fragment specifier
--> $DIR/issue-39404.rs:3:19
|
LL | macro_rules! m { ($i) => {} }
| ^^

error: aborting due to previous error

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

Expand Down