Skip to content

Commit 02eae43

Browse files
matkladmatklad
committed
Promote missing_fragment_specifier to hard error
It has been deny_by_default since 2017 (and warned for some time before that), so it seems reasonable to promote it. The specific technical motivation to do this now is to remove a field from `ParseSess` -- it is a global state, and global state makes extracting libraries annoying. Closes rust-lang#40107
1 parent 84fcd0d commit 02eae43

14 files changed

+32
-90
lines changed

‎src/librustc_expand/mbe.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ enum TokenTree {
8484
/// e.g., `$var`
8585
MetaVar(Span, Ident),
8686
/// e.g., `$var:expr`. This is only used in the left hand side of MBE macros.
87-
MetaVarDecl(Span, Ident /* name to bind */, Option<NonterminalKind>),
87+
MetaVarDecl(Span, Ident /* name to bind */, NonterminalKind),
8888
}
8989

9090
impl TokenTree {

‎src/librustc_expand/mbe/macro_parser.rs

+3-17
Original file line numberDiff line numberDiff line change
@@ -378,11 +378,6 @@ fn nameize<I: Iterator<Item = NamedMatch>>(
378378
n_rec(sess, next_m, res.by_ref(), ret_val)?;
379379
}
380380
}
381-
TokenTree::MetaVarDecl(span, _, None) => {
382-
if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
383-
return Err((span, "missing fragment specifier".to_string()));
384-
}
385-
}
386381
TokenTree::MetaVarDecl(sp, bind_name, _) => match ret_val
387382
.entry(MacroRulesNormalizedIdent::new(bind_name))
388383
{
@@ -442,7 +437,6 @@ fn token_name_eq(t1: &Token, t2: &Token) -> bool {
442437
///
443438
/// A `ParseResult`. Note that matches are kept track of through the items generated.
444439
fn inner_parse_loop<'root, 'tt>(
445-
sess: &ParseSess,
446440
cur_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
447441
next_items: &mut Vec<MatcherPosHandle<'root, 'tt>>,
448442
eof_items: &mut SmallVec<[MatcherPosHandle<'root, 'tt>; 1]>,
@@ -560,16 +554,9 @@ fn inner_parse_loop<'root, 'tt>(
560554
})));
561555
}
562556

563-
// We need to match a metavar (but the identifier is invalid)... this is an error
564-
TokenTree::MetaVarDecl(span, _, None) => {
565-
if sess.missing_fragment_specifiers.borrow_mut().remove(&span).is_some() {
566-
return Error(span, "missing fragment specifier".to_string());
567-
}
568-
}
569-
570557
// We need to match a metavar with a valid ident... call out to the black-box
571558
// parser by adding an item to `bb_items`.
572-
TokenTree::MetaVarDecl(_, _, Some(kind)) => {
559+
TokenTree::MetaVarDecl(_, _, kind) => {
573560
// Built-in nonterminals never start with these tokens,
574561
// so we can eliminate them from consideration.
575562
if Parser::nonterminal_may_begin_with(kind, token) {
@@ -640,7 +627,6 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na
640627
// parsing from the black-box parser done. The result is that `next_items` will contain a
641628
// bunch of possible next matcher positions in `next_items`.
642629
match inner_parse_loop(
643-
parser.sess,
644630
&mut cur_items,
645631
&mut next_items,
646632
&mut eof_items,
@@ -702,7 +688,7 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na
702688
let nts = bb_items
703689
.iter()
704690
.map(|item| match item.top_elts.get_tt(item.idx) {
705-
TokenTree::MetaVarDecl(_, bind, Some(kind)) => format!("{} ('{}')", kind, bind),
691+
TokenTree::MetaVarDecl(_, bind, kind) => format!("{} ('{}')", kind, bind),
706692
_ => panic!(),
707693
})
708694
.collect::<Vec<String>>()
@@ -732,7 +718,7 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na
732718
assert_eq!(bb_items.len(), 1);
733719

734720
let mut item = bb_items.pop().unwrap();
735-
if let TokenTree::MetaVarDecl(span, _, Some(kind)) = item.top_elts.get_tt(item.idx) {
721+
if let TokenTree::MetaVarDecl(span, _, kind) = item.top_elts.get_tt(item.idx) {
736722
let match_cur = item.match_cur;
737723
let nt = match parser.to_mut().parse_nonterminal(kind) {
738724
Err(mut err) => {

‎src/librustc_expand/mbe/macro_rules.rs

+7-8
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ pub fn compile_declarative_macro(
400400
let diag = &sess.parse_sess.span_diagnostic;
401401
let lhs_nm = Ident::new(sym::lhs, def.span);
402402
let rhs_nm = Ident::new(sym::rhs, def.span);
403-
let tt_spec = Some(NonterminalKind::TT);
403+
let tt_spec = NonterminalKind::TT;
404404

405405
// Parse the macro_rules! invocation
406406
let (macro_rules, body) = match &def.kind {
@@ -577,7 +577,7 @@ fn check_lhs_no_empty_seq(sess: &ParseSess, tts: &[mbe::TokenTree]) -> bool {
577577
TokenTree::Sequence(span, ref seq) => {
578578
if seq.separator.is_none()
579579
&& seq.tts.iter().all(|seq_tt| match *seq_tt {
580-
TokenTree::MetaVarDecl(_, _, Some(NonterminalKind::Vis)) => true,
580+
TokenTree::MetaVarDecl(_, _, NonterminalKind::Vis) => true,
581581
TokenTree::Sequence(_, ref sub_seq) => {
582582
sub_seq.kleene.op == mbe::KleeneOp::ZeroOrMore
583583
|| sub_seq.kleene.op == mbe::KleeneOp::ZeroOrOne
@@ -960,7 +960,7 @@ fn check_matcher_core(
960960
// Now `last` holds the complete set of NT tokens that could
961961
// end the sequence before SUFFIX. Check that every one works with `suffix`.
962962
for token in &last.tokens {
963-
if let TokenTree::MetaVarDecl(_, name, Some(kind)) = *token {
963+
if let TokenTree::MetaVarDecl(_, name, kind) = *token {
964964
for next_token in &suffix_first.tokens {
965965
match is_in_follow(next_token, kind) {
966966
IsInFollow::Yes => {}
@@ -1018,7 +1018,7 @@ fn check_matcher_core(
10181018
}
10191019

10201020
fn token_can_be_followed_by_any(tok: &mbe::TokenTree) -> bool {
1021-
if let mbe::TokenTree::MetaVarDecl(_, _, Some(kind)) = *tok {
1021+
if let mbe::TokenTree::MetaVarDecl(_, _, kind) = *tok {
10221022
frag_can_be_followed_by_any(kind)
10231023
} else {
10241024
// (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 {
11231123
}
11241124
_ => IsInFollow::No(TOKENS),
11251125
},
1126-
TokenTree::MetaVarDecl(_, _, Some(NonterminalKind::Block)) => IsInFollow::Yes,
1126+
TokenTree::MetaVarDecl(_, _, NonterminalKind::Block) => IsInFollow::Yes,
11271127
_ => IsInFollow::No(TOKENS),
11281128
}
11291129
}
@@ -1158,7 +1158,7 @@ fn is_in_follow(tok: &mbe::TokenTree, kind: NonterminalKind) -> IsInFollow {
11581158
TokenTree::MetaVarDecl(
11591159
_,
11601160
_,
1161-
Some(NonterminalKind::Ident | NonterminalKind::Ty | NonterminalKind::Path),
1161+
NonterminalKind::Ident | NonterminalKind::Ty | NonterminalKind::Path,
11621162
) => IsInFollow::Yes,
11631163
_ => IsInFollow::No(TOKENS),
11641164
}
@@ -1171,8 +1171,7 @@ fn quoted_tt_to_string(tt: &mbe::TokenTree) -> String {
11711171
match *tt {
11721172
mbe::TokenTree::Token(ref token) => pprust::token_to_string(&token),
11731173
mbe::TokenTree::MetaVar(_, name) => format!("${}", name),
1174-
mbe::TokenTree::MetaVarDecl(_, name, Some(kind)) => format!("${}:{}", name, kind),
1175-
mbe::TokenTree::MetaVarDecl(_, name, None) => format!("${}:", name),
1174+
mbe::TokenTree::MetaVarDecl(_, name, kind) => format!("${}:{}", name, kind),
11761175
_ => panic!(
11771176
"unexpected mbe::TokenTree::{{Sequence or Delimited}} \
11781177
in follow set checker"

‎src/librustc_expand/mbe/quoted.rs

+4-7
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::mbe::{Delimited, KleeneOp, KleeneToken, SequenceRepetition, TokenTree
33

44
use rustc_ast::token::{self, Token};
55
use rustc_ast::tokenstream;
6-
use rustc_ast::{NodeId, DUMMY_NODE_ID};
6+
use rustc_ast::NodeId;
77
use rustc_ast_pretty::pprust;
88
use rustc_session::parse::ParseSess;
99
use rustc_span::symbol::{kw, Ident};
@@ -73,7 +73,7 @@ pub(super) fn parse(
7373
.emit();
7474
token::NonterminalKind::Ident
7575
});
76-
result.push(TokenTree::MetaVarDecl(span, ident, Some(kind)));
76+
result.push(TokenTree::MetaVarDecl(span, ident, kind));
7777
continue;
7878
}
7979
_ => token.span,
@@ -83,11 +83,8 @@ pub(super) fn parse(
8383
}
8484
tree => tree.as_ref().map(tokenstream::TokenTree::span).unwrap_or(start_sp),
8585
};
86-
if node_id != DUMMY_NODE_ID {
87-
// Macros loaded from other crates have dummy node ids.
88-
sess.missing_fragment_specifiers.borrow_mut().insert(span, node_id);
89-
}
90-
result.push(TokenTree::MetaVarDecl(span, ident, None));
86+
sess.span_diagnostic.struct_span_err(span, "missing fragment specifier").emit();
87+
continue;
9188
}
9289

9390
// Not a metavar or no matchers allowed, so just return the tree

‎src/librustc_interface/passes.rs

+1-18
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ use rustc_passes::{self, hir_stats, layout_test};
3030
use rustc_plugin_impl as plugin;
3131
use rustc_resolve::{Resolver, ResolverArenas};
3232
use rustc_session::config::{CrateType, Input, OutputFilenames, OutputType, PpMode, PpSourceMode};
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::Session;
@@ -307,27 +306,11 @@ fn configure_and_expand_inner<'a>(
307306
ecx.check_unused_macros();
308307
});
309308

310-
let mut missing_fragment_specifiers: Vec<_> = ecx
311-
.sess
312-
.parse_sess
313-
.missing_fragment_specifiers
314-
.borrow()
315-
.iter()
316-
.map(|(span, node_id)| (*span, *node_id))
317-
.collect();
318-
missing_fragment_specifiers.sort_unstable_by_key(|(span, _)| *span);
319-
320-
let recursion_limit_hit = ecx.reduced_recursion_limit.is_some();
321-
322-
for (span, node_id) in missing_fragment_specifiers {
323-
let lint = lint::builtin::MISSING_FRAGMENT_SPECIFIER;
324-
let msg = "missing fragment specifier";
325-
resolver.lint_buffer().buffer_lint(lint, node_id, span, msg);
326-
}
327309
if cfg!(windows) {
328310
env::set_var("PATH", &old_path);
329311
}
330312

313+
let recursion_limit_hit = ecx.reduced_recursion_limit.is_some();
331314
if recursion_limit_hit {
332315
// If we hit a recursion limit, exit early to avoid later passes getting overwhelmed
333316
// with a large AST

‎src/librustc_session/parse.rs

-2
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ pub struct ParseSess {
119119
pub unstable_features: UnstableFeatures,
120120
pub config: CrateConfig,
121121
pub edition: Edition,
122-
pub missing_fragment_specifiers: Lock<FxHashMap<Span, NodeId>>,
123122
/// Places where raw identifiers were used. This is used for feature-gating raw identifiers.
124123
pub raw_identifier_spans: Lock<Vec<Span>>,
125124
/// Used to determine and report recursive module inclusions.
@@ -154,7 +153,6 @@ impl ParseSess {
154153
unstable_features: UnstableFeatures::from_environment(),
155154
config: FxHashSet::default(),
156155
edition: ExpnId::root().expn_data().edition,
157-
missing_fragment_specifiers: Default::default(),
158156
raw_identifier_spans: Lock::new(Vec::new()),
159157
included_mod_stack: Lock::new(vec![]),
160158
source_map,

‎src/test/ui/lint/expansion-time.rs

-4
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,6 @@ macro_rules! foo {
55
( $($i:ident)* ) => { $($i)+ }; //~ WARN meta-variable repeats with different Kleene operator
66
}
77

8-
#[warn(missing_fragment_specifier)]
9-
macro_rules! m { ($i) => {} } //~ WARN missing fragment specifier
10-
//~| WARN this was previously accepted
11-
128
#[warn(soft_unstable)]
139
mod benches {
1410
#[bench] //~ WARN use of unstable library feature 'test'

‎src/test/ui/lint/expansion-time.stderr

+4-18
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,14 @@ note: the lint level is defined here
1212
LL | #[warn(meta_variable_misuse)]
1313
| ^^^^^^^^^^^^^^^^^^^^
1414

15-
warning: missing fragment specifier
16-
--> $DIR/expansion-time.rs:9:19
17-
|
18-
LL | macro_rules! m { ($i) => {} }
19-
| ^^
20-
|
21-
note: the lint level is defined here
22-
--> $DIR/expansion-time.rs:8:8
23-
|
24-
LL | #[warn(missing_fragment_specifier)]
25-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
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-
2915
warning: use of unstable library feature 'test': `bench` is a part of custom test frameworks which are unstable
30-
--> $DIR/expansion-time.rs:14:7
16+
--> $DIR/expansion-time.rs:10:7
3117
|
3218
LL | #[bench]
3319
| ^^^^^
3420
|
3521
note: the lint level is defined here
36-
--> $DIR/expansion-time.rs:12:8
22+
--> $DIR/expansion-time.rs:8:8
3723
|
3824
LL | #[warn(soft_unstable)]
3925
| ^^^^^^^^^^^^^
@@ -47,10 +33,10 @@ LL | 2
4733
| ^
4834
|
4935
note: the lint level is defined here
50-
--> $DIR/expansion-time.rs:19:8
36+
--> $DIR/expansion-time.rs:15:8
5137
|
5238
LL | #[warn(incomplete_include)]
5339
| ^^^^^^^^^^^^^^^^^^
5440

55-
warning: 4 warnings emitted
41+
warning: 3 warnings emitted
5642

‎src/test/ui/macros/issue-39404.rs

-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,5 @@
22

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

76
fn main() {}

‎src/test/ui/macros/issue-39404.stderr

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

117
error: aborting due to previous error
128

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

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

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

-4
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@ error: missing fragment specifier
99
|
1010
LL | ($a, $b) => {
1111
| ^^
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>
1612

1713
error: aborting due to 2 previous errors
1814

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

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ macro_rules! foo {
22
{ $+ } => { //~ ERROR expected identifier, found `+`
33
//~^ ERROR missing fragment specifier
44
$(x)(y) //~ ERROR expected one of: `*`, `+`, or `?`
5+
//~^ ERROR attempted to repeat an expression containing no syntax variables
56
}
67
}
78

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

+11-5
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,23 @@ error: expected identifier, found `+`
44
LL | { $+ } => {
55
| ^
66

7+
error: missing fragment specifier
8+
--> $DIR/issue-33569.rs:2:8
9+
|
10+
LL | { $+ } => {
11+
| ^
12+
713
error: expected one of: `*`, `+`, or `?`
814
--> $DIR/issue-33569.rs:4:13
915
|
1016
LL | $(x)(y)
1117
| ^^^
1218

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

19-
error: aborting due to 3 previous errors
25+
error: aborting due to 4 previous errors
2026

0 commit comments

Comments
 (0)