Skip to content

expand: Remove ParseSess::missing_fragment_specifiers #95808

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

Merged
merged 1 commit into from
Apr 9, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions compiler/rustc_expand/src/mbe/macro_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ use rustc_ast::token::{DelimToken, Token, TokenKind};
use rustc_ast::{NodeId, DUMMY_NODE_ID};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::MultiSpan;
use rustc_session::lint::builtin::META_VARIABLE_MISUSE;
use rustc_session::lint::builtin::{META_VARIABLE_MISUSE, MISSING_FRAGMENT_SPECIFIER};
use rustc_session::parse::ParseSess;
use rustc_span::symbol::kw;
use rustc_span::{symbol::MacroRulesNormalizedIdent, Span};
Expand Down Expand Up @@ -261,7 +261,18 @@ fn check_binders(
}
}
// Similarly, this can only happen when checking a toplevel macro.
TokenTree::MetaVarDecl(span, name, _kind) => {
TokenTree::MetaVarDecl(span, name, kind) => {
if kind.is_none() && node_id != DUMMY_NODE_ID {
// FIXME: Report this as a hard error eventually and remove equivalent errors from
// `parse_tt_inner` and `nameize`. Until then the error may be reported twice, once
// as a hard error and then once as a buffered lint.
sess.buffer_lint(
MISSING_FRAGMENT_SPECIFIER,
span,
node_id,
&format!("missing fragment specifier"),
);
}
if !macros.is_empty() {
sess.span_diagnostic.span_bug(span, "unexpected MetaVarDecl in nested lhs");
}
Expand Down
18 changes: 6 additions & 12 deletions compiler/rustc_expand/src/mbe/macro_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,6 @@ impl TtParser {
/// track of through the mps generated.
fn parse_tt_inner(
&mut self,
sess: &ParseSess,
matcher: &[MatcherLoc],
token: &Token,
) -> Option<NamedParseResult> {
Expand Down Expand Up @@ -519,11 +518,9 @@ impl TtParser {
self.bb_mps.push(mp);
}
} else {
// E.g. `$e` instead of `$e:expr`, reported as a hard error if actually used.
// Both this check and the one in `nameize` are necessary, surprisingly.
if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
// E.g. `$e` instead of `$e:expr`.
return Some(Error(span, "missing fragment specifier".to_string()));
}
return Some(Error(span, "missing fragment specifier".to_string()));
}
}
MatcherLoc::Eof => {
Expand All @@ -549,7 +546,7 @@ impl TtParser {
// Need to take ownership of the matches from within the `Lrc`.
Lrc::make_mut(&mut eof_mp.matches);
let matches = Lrc::try_unwrap(eof_mp.matches).unwrap().into_iter();
self.nameize(sess, matcher, matches)
self.nameize(matcher, matches)
}
EofMatcherPositions::Multiple => {
Error(token.span, "ambiguity: multiple successful parses".to_string())
Expand Down Expand Up @@ -587,7 +584,7 @@ impl TtParser {

// Process `cur_mps` until either we have finished the input or we need to get some
// parsing from the black-box parser done.
if let Some(res) = self.parse_tt_inner(&parser.sess, matcher, &parser.token) {
if let Some(res) = self.parse_tt_inner(matcher, &parser.token) {
return res;
}

Expand Down Expand Up @@ -694,7 +691,6 @@ impl TtParser {

fn nameize<I: Iterator<Item = NamedMatch>>(
&self,
sess: &ParseSess,
matcher: &[MatcherLoc],
mut res: I,
) -> NamedParseResult {
Expand All @@ -711,11 +707,9 @@ impl TtParser {
}
};
} else {
// E.g. `$e` instead of `$e:expr`, reported as a hard error if actually used.
// Both this check and the one in `parse_tt_inner` are necessary, surprisingly.
if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
// E.g. `$e` instead of `$e:expr`.
return Error(span, "missing fragment specifier".to_string());
}
return Error(span, "missing fragment specifier".to_string());
}
}
}
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_expand/src/mbe/quoted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use crate::mbe::macro_parser::count_metavar_decls;
use crate::mbe::{Delimited, KleeneOp, KleeneToken, MetaVarExpr, SequenceRepetition, TokenTree};

use rustc_ast::token::{self, Token};
use rustc_ast::tokenstream;
use rustc_ast::{NodeId, DUMMY_NODE_ID};
use rustc_ast::{tokenstream, NodeId};
use rustc_ast_pretty::pprust;
use rustc_feature::Features;
use rustc_session::parse::{feature_err, ParseSess};
Expand Down Expand Up @@ -104,10 +103,7 @@ pub(super) fn parse(
}
tree => tree.as_ref().map_or(start_sp, tokenstream::TokenTree::span),
};
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));
}

Expand Down
16 changes: 0 additions & 16 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ use rustc_resolve::{Resolver, ResolverArenas};
use rustc_serialize::json;
use rustc_session::config::{CrateType, Input, OutputFilenames, OutputType};
use rustc_session::cstore::{MetadataLoader, MetadataLoaderDyn};
use rustc_session::lint;
use rustc_session::output::{filename_for_input, filename_for_metadata};
use rustc_session::search_paths::PathKind;
use rustc_session::{Limit, Session};
Expand Down Expand Up @@ -349,23 +348,8 @@ pub fn configure_and_expand(
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);
}
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_session/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ pub struct ParseSess {
pub config: CrateConfig,
pub check_config: CrateCheckConfig,
pub edition: Edition,
pub missing_fragment_specifiers: Lock<FxHashMap<Span, NodeId>>,
/// Places where raw identifiers were used. This is used to avoid complaining about idents
/// clashing with keywords in new editions.
pub raw_identifier_spans: Lock<Vec<Span>>,
Expand Down Expand Up @@ -195,7 +194,6 @@ impl ParseSess {
config: FxHashSet::default(),
check_config: CrateCheckConfig::default(),
edition: ExpnId::root().expn_data().edition,
missing_fragment_specifiers: Default::default(),
raw_identifier_spans: Lock::new(Vec::new()),
bad_unicode_identifiers: Lock::new(Default::default()),
source_map,
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/macros/macro-match-nonterminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ macro_rules! test {
($a, $b) => {
//~^ ERROR missing fragment
//~| ERROR missing fragment
//~| ERROR missing fragment
//~| WARN this was previously accepted
//~| WARN this was previously accepted
()
};
Expand Down
13 changes: 11 additions & 2 deletions src/test/ui/macros/macro-match-nonterminal.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,24 @@ error: missing fragment specifier
LL | ($a, $b) => {
| ^

error: missing fragment specifier
--> $DIR/macro-match-nonterminal.rs:2:8
|
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: missing fragment specifier
--> $DIR/macro-match-nonterminal.rs:2:10
|
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 3 previous errors

15 changes: 15 additions & 0 deletions src/test/ui/macros/macro-missing-fragment-deduplication.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// compile-flags: -Zdeduplicate-diagnostics=yes

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

fn main() {
m!();
m!();
m!();
m!();
}
18 changes: 18 additions & 0 deletions src/test/ui/macros/macro-missing-fragment-deduplication.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: missing fragment specifier
--> $DIR/macro-missing-fragment-deduplication.rs:4:6
|
LL | ($name) => {}
| ^^^^^

error: missing fragment specifier
--> $DIR/macro-missing-fragment-deduplication.rs:4:6
|
LL | ($name) => {}
| ^^^^^
|
= 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

25 changes: 22 additions & 3 deletions src/test/ui/macros/macro-missing-fragment.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,26 @@
macro_rules! m {
( $( any_token $field_rust_type )* ) => {}; //~ ERROR missing fragment
#![warn(missing_fragment_specifier)]

macro_rules! used_arm {
( $( any_token $field_rust_type )* ) => {};
//~^ ERROR missing fragment
//~| WARN missing fragment
//~| WARN this was previously accepted
}

macro_rules! used_macro_unused_arm {
() => {};
( $name ) => {};
//~^ WARN missing fragment
//~| WARN this was previously accepted
}

macro_rules! unused_macro {
( $name ) => {};
//~^ WARN missing fragment
//~| WARN this was previously accepted
}

fn main() {
m!();
used_arm!();
used_macro_unused_arm!();
}
36 changes: 34 additions & 2 deletions src/test/ui/macros/macro-missing-fragment.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,40 @@
error: missing fragment specifier
--> $DIR/macro-missing-fragment.rs:2:20
--> $DIR/macro-missing-fragment.rs:4:20
|
LL | ( $( any_token $field_rust_type )* ) => {};
| ^^^^^^^^^^^^^^^^

error: aborting due to previous error
warning: missing fragment specifier
--> $DIR/macro-missing-fragment.rs:4:20
|
LL | ( $( any_token $field_rust_type )* ) => {};
| ^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/macro-missing-fragment.rs:1:9
|
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: missing fragment specifier
--> $DIR/macro-missing-fragment.rs:12:7
|
LL | ( $name ) => {};
| ^^^^^
|
= 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: missing fragment specifier
--> $DIR/macro-missing-fragment.rs:18:7
|
LL | ( $name ) => {};
| ^^^^^
|
= 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; 3 warnings emitted

2 changes: 2 additions & 0 deletions src/test/ui/parser/macro/issue-33569.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
macro_rules! foo {
{ $+ } => { //~ ERROR expected identifier, found `+`
//~^ ERROR missing fragment specifier
//~| ERROR missing fragment specifier
//~| WARN this was previously accepted
$(x)(y) //~ ERROR expected one of: `*`, `+`, or `?`
}
}
Expand Down
14 changes: 12 additions & 2 deletions src/test/ui/parser/macro/issue-33569.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | { $+ } => {
| ^

error: expected one of: `*`, `+`, or `?`
--> $DIR/issue-33569.rs:4:13
--> $DIR/issue-33569.rs:6:13
|
LL | $(x)(y)
| ^^^
Expand All @@ -16,5 +16,15 @@ error: missing fragment specifier
LL | { $+ } => {
| ^

error: aborting due to 3 previous errors
error: missing fragment specifier
--> $DIR/issue-33569.rs:2:8
|
LL | { $+ } => {
| ^
|
= 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 4 previous errors