Skip to content

Commit 17157c7

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of #95808 - petrochenkov:fragspec, r=nnethercote
expand: Remove `ParseSess::missing_fragment_specifiers` It was used for deduplicating some errors for legacy code which are mostly deduplicated even without that, but at cost of global mutable state, which is not a good tradeoff. cc #95747 (comment) r? ``@nnethercote``
2 parents 5092946 + 379ae12 commit 17157c7

13 files changed

+137
-47
lines changed

‎compiler/rustc_expand/src/mbe/macro_check.rs

+13-2
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ use rustc_ast::token::{DelimToken, Token, TokenKind};
110110
use rustc_ast::{NodeId, DUMMY_NODE_ID};
111111
use rustc_data_structures::fx::FxHashMap;
112112
use rustc_errors::MultiSpan;
113-
use rustc_session::lint::builtin::META_VARIABLE_MISUSE;
113+
use rustc_session::lint::builtin::{META_VARIABLE_MISUSE, MISSING_FRAGMENT_SPECIFIER};
114114
use rustc_session::parse::ParseSess;
115115
use rustc_span::symbol::kw;
116116
use rustc_span::{symbol::MacroRulesNormalizedIdent, Span};
@@ -261,7 +261,18 @@ fn check_binders(
261261
}
262262
}
263263
// Similarly, this can only happen when checking a toplevel macro.
264-
TokenTree::MetaVarDecl(span, name, _kind) => {
264+
TokenTree::MetaVarDecl(span, name, kind) => {
265+
if kind.is_none() && node_id != DUMMY_NODE_ID {
266+
// FIXME: Report this as a hard error eventually and remove equivalent errors from
267+
// `parse_tt_inner` and `nameize`. Until then the error may be reported twice, once
268+
// as a hard error and then once as a buffered lint.
269+
sess.buffer_lint(
270+
MISSING_FRAGMENT_SPECIFIER,
271+
span,
272+
node_id,
273+
&format!("missing fragment specifier"),
274+
);
275+
}
265276
if !macros.is_empty() {
266277
sess.span_diagnostic.span_bug(span, "unexpected MetaVarDecl in nested lhs");
267278
}

‎compiler/rustc_expand/src/mbe/macro_parser.rs

+6-12
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,6 @@ impl TtParser {
411411
/// track of through the mps generated.
412412
fn parse_tt_inner(
413413
&mut self,
414-
sess: &ParseSess,
415414
matcher: &[MatcherLoc],
416415
token: &Token,
417416
) -> Option<NamedParseResult> {
@@ -519,11 +518,9 @@ impl TtParser {
519518
self.bb_mps.push(mp);
520519
}
521520
} else {
521+
// E.g. `$e` instead of `$e:expr`, reported as a hard error if actually used.
522522
// Both this check and the one in `nameize` are necessary, surprisingly.
523-
if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
524-
// E.g. `$e` instead of `$e:expr`.
525-
return Some(Error(span, "missing fragment specifier".to_string()));
526-
}
523+
return Some(Error(span, "missing fragment specifier".to_string()));
527524
}
528525
}
529526
MatcherLoc::Eof => {
@@ -549,7 +546,7 @@ impl TtParser {
549546
// Need to take ownership of the matches from within the `Lrc`.
550547
Lrc::make_mut(&mut eof_mp.matches);
551548
let matches = Lrc::try_unwrap(eof_mp.matches).unwrap().into_iter();
552-
self.nameize(sess, matcher, matches)
549+
self.nameize(matcher, matches)
553550
}
554551
EofMatcherPositions::Multiple => {
555552
Error(token.span, "ambiguity: multiple successful parses".to_string())
@@ -587,7 +584,7 @@ impl TtParser {
587584

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

@@ -694,7 +691,6 @@ impl TtParser {
694691

695692
fn nameize<I: Iterator<Item = NamedMatch>>(
696693
&self,
697-
sess: &ParseSess,
698694
matcher: &[MatcherLoc],
699695
mut res: I,
700696
) -> NamedParseResult {
@@ -711,11 +707,9 @@ impl TtParser {
711707
}
712708
};
713709
} else {
710+
// E.g. `$e` instead of `$e:expr`, reported as a hard error if actually used.
714711
// Both this check and the one in `parse_tt_inner` are necessary, surprisingly.
715-
if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
716-
// E.g. `$e` instead of `$e:expr`.
717-
return Error(span, "missing fragment specifier".to_string());
718-
}
712+
return Error(span, "missing fragment specifier".to_string());
719713
}
720714
}
721715
}

‎compiler/rustc_expand/src/mbe/quoted.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ use crate::mbe::macro_parser::count_metavar_decls;
22
use crate::mbe::{Delimited, KleeneOp, KleeneToken, MetaVarExpr, SequenceRepetition, TokenTree};
33

44
use rustc_ast::token::{self, Token};
5-
use rustc_ast::tokenstream;
6-
use rustc_ast::{NodeId, DUMMY_NODE_ID};
5+
use rustc_ast::{tokenstream, NodeId};
76
use rustc_ast_pretty::pprust;
87
use rustc_feature::Features;
98
use rustc_session::parse::{feature_err, ParseSess};
@@ -104,10 +103,7 @@ pub(super) fn parse(
104103
}
105104
tree => tree.as_ref().map_or(start_sp, tokenstream::TokenTree::span),
106105
};
107-
if node_id != DUMMY_NODE_ID {
108-
// Macros loaded from other crates have dummy node ids.
109-
sess.missing_fragment_specifiers.borrow_mut().insert(span, node_id);
110-
}
106+
111107
result.push(TokenTree::MetaVarDecl(span, ident, None));
112108
}
113109

‎compiler/rustc_interface/src/passes.rs

-16
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ use rustc_resolve::{Resolver, ResolverArenas};
3030
use rustc_serialize::json;
3131
use rustc_session::config::{CrateType, Input, OutputFilenames, OutputType};
3232
use rustc_session::cstore::{MetadataLoader, MetadataLoaderDyn};
33-
use rustc_session::lint;
3433
use rustc_session::output::{filename_for_input, filename_for_metadata};
3534
use rustc_session::search_paths::PathKind;
3635
use rustc_session::{Limit, Session};
@@ -349,23 +348,8 @@ pub fn configure_and_expand(
349348
ecx.check_unused_macros();
350349
});
351350

352-
let mut missing_fragment_specifiers: Vec<_> = ecx
353-
.sess
354-
.parse_sess
355-
.missing_fragment_specifiers
356-
.borrow()
357-
.iter()
358-
.map(|(span, node_id)| (*span, *node_id))
359-
.collect();
360-
missing_fragment_specifiers.sort_unstable_by_key(|(span, _)| *span);
361-
362351
let recursion_limit_hit = ecx.reduced_recursion_limit.is_some();
363352

364-
for (span, node_id) in missing_fragment_specifiers {
365-
let lint = lint::builtin::MISSING_FRAGMENT_SPECIFIER;
366-
let msg = "missing fragment specifier";
367-
resolver.lint_buffer().buffer_lint(lint, node_id, span, msg);
368-
}
369353
if cfg!(windows) {
370354
env::set_var("PATH", &old_path);
371355
}

‎compiler/rustc_session/src/parse.rs

-2
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ pub struct ParseSess {
140140
pub config: CrateConfig,
141141
pub check_config: CrateCheckConfig,
142142
pub edition: Edition,
143-
pub missing_fragment_specifiers: Lock<FxHashMap<Span, NodeId>>,
144143
/// Places where raw identifiers were used. This is used to avoid complaining about idents
145144
/// clashing with keywords in new editions.
146145
pub raw_identifier_spans: Lock<Vec<Span>>,
@@ -195,7 +194,6 @@ impl ParseSess {
195194
config: FxHashSet::default(),
196195
check_config: CrateCheckConfig::default(),
197196
edition: ExpnId::root().expn_data().edition,
198-
missing_fragment_specifiers: Default::default(),
199197
raw_identifier_spans: Lock::new(Vec::new()),
200198
bad_unicode_identifiers: Lock::new(Default::default()),
201199
source_map,

‎src/test/ui/macros/macro-match-nonterminal.rs

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ macro_rules! test {
22
($a, $b) => {
33
//~^ ERROR missing fragment
44
//~| ERROR missing fragment
5+
//~| ERROR missing fragment
6+
//~| WARN this was previously accepted
57
//~| WARN this was previously accepted
68
()
79
};

‎src/test/ui/macros/macro-match-nonterminal.stderr

+11-2
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,24 @@ error: missing fragment specifier
44
LL | ($a, $b) => {
55
| ^
66

7+
error: missing fragment specifier
8+
--> $DIR/macro-match-nonterminal.rs:2:8
9+
|
10+
LL | ($a, $b) => {
11+
| ^
12+
|
13+
= note: `#[deny(missing_fragment_specifier)]` on by default
14+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
15+
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
16+
717
error: missing fragment specifier
818
--> $DIR/macro-match-nonterminal.rs:2:10
919
|
1020
LL | ($a, $b) => {
1121
| ^^
1222
|
13-
= note: `#[deny(missing_fragment_specifier)]` on by default
1423
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
1524
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
1625

17-
error: aborting due to 2 previous errors
26+
error: aborting due to 3 previous errors
1827

‎src/test/ui/macros/macro-missing-fragment-deduplication.rs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// compile-flags: -Zdeduplicate-diagnostics=yes
2+
3+
macro_rules! m {
4+
($name) => {}
5+
//~^ ERROR missing fragment
6+
//~| ERROR missing fragment
7+
//~| WARN this was previously accepted
8+
}
9+
10+
fn main() {
11+
m!();
12+
m!();
13+
m!();
14+
m!();
15+
}

‎src/test/ui/macros/macro-missing-fragment-deduplication.stderr

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: missing fragment specifier
2+
--> $DIR/macro-missing-fragment-deduplication.rs:4:6
3+
|
4+
LL | ($name) => {}
5+
| ^^^^^
6+
7+
error: missing fragment specifier
8+
--> $DIR/macro-missing-fragment-deduplication.rs:4:6
9+
|
10+
LL | ($name) => {}
11+
| ^^^^^
12+
|
13+
= note: `#[deny(missing_fragment_specifier)]` on by default
14+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
15+
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
16+
17+
error: aborting due to 2 previous errors
18+

‎src/test/ui/macros/macro-missing-fragment.rs

+22-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,26 @@
1-
macro_rules! m {
2-
( $( any_token $field_rust_type )* ) => {}; //~ ERROR missing fragment
1+
#![warn(missing_fragment_specifier)]
2+
3+
macro_rules! used_arm {
4+
( $( any_token $field_rust_type )* ) => {};
5+
//~^ ERROR missing fragment
6+
//~| WARN missing fragment
7+
//~| WARN this was previously accepted
8+
}
9+
10+
macro_rules! used_macro_unused_arm {
11+
() => {};
12+
( $name ) => {};
13+
//~^ WARN missing fragment
14+
//~| WARN this was previously accepted
15+
}
16+
17+
macro_rules! unused_macro {
18+
( $name ) => {};
19+
//~^ WARN missing fragment
20+
//~| WARN this was previously accepted
321
}
422

523
fn main() {
6-
m!();
24+
used_arm!();
25+
used_macro_unused_arm!();
726
}

‎src/test/ui/macros/macro-missing-fragment.stderr

+34-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,40 @@
11
error: missing fragment specifier
2-
--> $DIR/macro-missing-fragment.rs:2:20
2+
--> $DIR/macro-missing-fragment.rs:4:20
33
|
44
LL | ( $( any_token $field_rust_type )* ) => {};
55
| ^^^^^^^^^^^^^^^^
66

7-
error: aborting due to previous error
7+
warning: missing fragment specifier
8+
--> $DIR/macro-missing-fragment.rs:4:20
9+
|
10+
LL | ( $( any_token $field_rust_type )* ) => {};
11+
| ^^^^^^^^^^^^^^^^
12+
|
13+
note: the lint level is defined here
14+
--> $DIR/macro-missing-fragment.rs:1:9
15+
|
16+
LL | #![warn(missing_fragment_specifier)]
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
18+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
19+
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
20+
21+
warning: missing fragment specifier
22+
--> $DIR/macro-missing-fragment.rs:12:7
23+
|
24+
LL | ( $name ) => {};
25+
| ^^^^^
26+
|
27+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
28+
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
29+
30+
warning: missing fragment specifier
31+
--> $DIR/macro-missing-fragment.rs:18:7
32+
|
33+
LL | ( $name ) => {};
34+
| ^^^^^
35+
|
36+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
37+
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
38+
39+
error: aborting due to previous error; 3 warnings emitted
840

‎src/test/ui/parser/macro/issue-33569.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
macro_rules! foo {
22
{ $+ } => { //~ ERROR expected identifier, found `+`
33
//~^ ERROR missing fragment specifier
4+
//~| ERROR missing fragment specifier
5+
//~| WARN this was previously accepted
46
$(x)(y) //~ ERROR expected one of: `*`, `+`, or `?`
57
}
68
}

‎src/test/ui/parser/macro/issue-33569.stderr

+12-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | { $+ } => {
55
| ^
66

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

19-
error: aborting due to 3 previous errors
19+
error: missing fragment specifier
20+
--> $DIR/issue-33569.rs:2:8
21+
|
22+
LL | { $+ } => {
23+
| ^
24+
|
25+
= note: `#[deny(missing_fragment_specifier)]` on by default
26+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
27+
= note: for more information, see issue #40107 <https://github.com/rust-lang/rust/issues/40107>
28+
29+
error: aborting due to 4 previous errors
2030

0 commit comments

Comments
 (0)