Rollup merge of #65974 - Centril:matcher-friendly-gating, r=petrochenkov

A scheme for more macro-matcher friendly pre-expansion gating

Pre-expansion gating will now avoid gating macro matchers that did not result in `Success(...)`. That is, the following is now OK despite `box 42` being a valid `expr` and that form being pre-expansion gated:

```rust
macro_rules! m {
    ($e:expr) => { 0 }; // This fails on the input below due to `, foo`.
    (box $e:expr, foo) => { 1 }; // Successful matcher, we should get `2`.
}

fn main() {
    assert_eq!(1, m!(box 42, foo));
}
```

Closes https://github.com/rust-lang/rust/issues/65846.

r? @petrochenkov
cc @Mark-Simulacrum

src/libsyntax/feature_gate/check.rs

@@ -870,18 +870,17 @@ pub fn check_crate(krate: &ast::Crate,
     maybe_stage_features(&parse_sess.span_diagnostic, krate, unstable);
     let mut visitor = PostExpansionVisitor { parse_sess, features };
 
+    let spans = parse_sess.gated_spans.spans.borrow();
     macro_rules! gate_all {
-        ($gate:ident, $msg:literal) => { gate_all!($gate, $gate, $msg); };
-        ($spans:ident, $gate:ident, $msg:literal) => {
-            for span in &*parse_sess.gated_spans.$spans.borrow() {
+        ($gate:ident, $msg:literal) => {
+            for span in spans.get(&sym::$gate).unwrap_or(&vec![]) {
                 gate_feature!(&visitor, $gate, *span, $msg);
             }
         }
     }
-
     gate_all!(let_chains, "`let` expressions in this position are experimental");
     gate_all!(async_closure, "async closures are unstable");
-    gate_all!(yields, generators, "yield syntax is experimental");
+    gate_all!(generators, "yield syntax is experimental");
     gate_all!(or_patterns, "or-patterns syntax is experimental");
     gate_all!(const_extern_fn, "`const extern fn` definitions are unstable");
 
@@ -892,7 +891,7 @@ pub fn check_crate(krate: &ast::Crate,
             // FIXME(eddyb) do something more useful than always
             // disabling these uses of early feature-gatings.
             if false {
-                for span in &*parse_sess.gated_spans.$gate.borrow() {
+                for span in spans.get(&sym::$gate).unwrap_or(&vec![]) {
                     gate_feature!(&visitor, $gate, *span, $msg);
                 }
             }
@@ -909,7 +908,6 @@ pub fn check_crate(krate: &ast::Crate,
     gate_all!(try_blocks, "`try` blocks are unstable");
     gate_all!(label_break_value, "labels on blocks are unstable");
     gate_all!(box_syntax, "box expression syntax is experimental; you can call `Box::new` instead");
-
     // To avoid noise about type ascription in common syntax errors,
     // only emit if it is the *only* error. (Also check it last.)
     if parse_sess.span_diagnostic.err_count() == 0 {

src/libsyntax/parse/parser.rs

@@ -1120,7 +1120,7 @@ impl<'a> Parser<'a> {
         self.expected_tokens.push(TokenType::Keyword(kw::Crate));
         if self.is_crate_vis() {
             self.bump(); // `crate`
-            self.sess.gated_spans.crate_visibility_modifier.borrow_mut().push(self.prev_span);
+            self.sess.gated_spans.gate(sym::crate_visibility_modifier, self.prev_span);
             return Ok(respan(self.prev_span, VisibilityKind::Crate(CrateSugar::JustCrate)));
         }
 

src/libsyntax/parse/parser/expr.rs

@@ -16,10 +16,10 @@ use crate::parse::token::{self, Token, TokenKind};
 use crate::print::pprust;
 use crate::ptr::P;
 use crate::source_map::{self, Span};
-use crate::symbol::{kw, sym};
 use crate::util::parser::{AssocOp, Fixity, prec_let_scrutinee_needs_par};
 
 use errors::Applicability;
+use syntax_pos::symbol::{kw, sym};
 use syntax_pos::Symbol;
 use std::mem;
 use rustc_data_structures::thin_vec::ThinVec;
@@ -252,7 +252,7 @@ impl<'a> Parser<'a> {
                 self.last_type_ascription = Some((self.prev_span, maybe_path));
 
                 lhs = self.parse_assoc_op_cast(lhs, lhs_span, ExprKind::Type)?;
-                self.sess.gated_spans.type_ascription.borrow_mut().push(lhs.span);
+                self.sess.gated_spans.gate(sym::type_ascription, lhs.span);
                 continue
             } else if op == AssocOp::DotDot || op == AssocOp::DotDotEq {
                 // If we didn’t have to handle `x..`/`x..=`, it would be pretty easy to
@@ -455,7 +455,7 @@ impl<'a> Parser<'a> {
                 let e = self.parse_prefix_expr(None);
                 let (span, e) = self.interpolated_or_expr_span(e)?;
                 let span = lo.to(span);
-                self.sess.gated_spans.box_syntax.borrow_mut().push(span);
+                self.sess.gated_spans.gate(sym::box_syntax, span);
                 (span, ExprKind::Box(e))
             }
             token::Ident(..) if self.token.is_ident_named(sym::not) => {
@@ -1045,7 +1045,7 @@ impl<'a> Parser<'a> {
                     }
 
                     let span = lo.to(hi);
-                    self.sess.gated_spans.yields.borrow_mut().push(span);
+                    self.sess.gated_spans.gate(sym::generators, span);
                 } else if self.eat_keyword(kw::Let) {
                     return self.parse_let_expr(attrs);
                 } else if is_span_rust_2018 && self.eat_keyword(kw::Await) {
@@ -1268,7 +1268,7 @@ impl<'a> Parser<'a> {
         outer_attrs: ThinVec<Attribute>,
     ) -> PResult<'a, P<Expr>> {
         if let Some(label) = opt_label {
-            self.sess.gated_spans.label_break_value.borrow_mut().push(label.ident.span);
+            self.sess.gated_spans.gate(sym::label_break_value, label.ident.span);
         }
 
         self.expect(&token::OpenDelim(token::Brace))?;
@@ -1297,7 +1297,7 @@ impl<'a> Parser<'a> {
         };
         if asyncness.is_async() {
             // Feature-gate `async ||` closures.
-            self.sess.gated_spans.async_closure.borrow_mut().push(self.prev_span);
+            self.sess.gated_spans.gate(sym::async_closure, self.prev_span);
         }
 
         let capture_clause = self.parse_capture_clause();
@@ -1419,8 +1419,7 @@ impl<'a> Parser<'a> {
 
         if let ExprKind::Let(..) = cond.kind {
             // Remove the last feature gating of a `let` expression since it's stable.
-            let last = self.sess.gated_spans.let_chains.borrow_mut().pop();
-            debug_assert_eq!(cond.span, last.unwrap());
+            self.sess.gated_spans.ungate_last(sym::let_chains, cond.span);
         }
 
         Ok(cond)
@@ -1437,7 +1436,7 @@ impl<'a> Parser<'a> {
             |this| this.parse_assoc_expr_with(1 + prec_let_scrutinee_needs_par(), None.into())
         )?;
         let span = lo.to(expr.span);
-        self.sess.gated_spans.let_chains.borrow_mut().push(span);
+        self.sess.gated_spans.gate(sym::let_chains, span);
         Ok(self.mk_expr(span, ExprKind::Let(pat, expr), attrs))
     }
 
@@ -1658,7 +1657,7 @@ impl<'a> Parser<'a> {
             Err(error)
         } else {
             let span = span_lo.to(body.span);
-            self.sess.gated_spans.try_blocks.borrow_mut().push(span);
+            self.sess.gated_spans.gate(sym::try_blocks, span);
             Ok(self.mk_expr(span, ExprKind::TryBlock(body), attrs))
         }
     }

src/libsyntax/parse/parser/generics.rs

@@ -3,7 +3,8 @@ use super::{Parser, PResult};
 use crate::ast::{self, WhereClause, GenericParam, GenericParamKind, GenericBounds, Attribute};
 use crate::parse::token;
 use crate::source_map::DUMMY_SP;
-use crate::symbol::kw;
+
+use syntax_pos::symbol::{kw, sym};
 
 impl<'a> Parser<'a> {
     /// Parses bounds of a lifetime parameter `BOUND + BOUND + BOUND`, possibly with trailing `+`.
@@ -62,7 +63,7 @@ impl<'a> Parser<'a> {
         self.expect(&token::Colon)?;
         let ty = self.parse_ty()?;
 
-        self.sess.gated_spans.const_generics.borrow_mut().push(lo.to(self.prev_span));
+        self.sess.gated_spans.gate(sym::const_generics, lo.to(self.prev_span));
 
         Ok(GenericParam {
             ident,

src/libsyntax/parse/parser/item.rs

@@ -146,9 +146,7 @@ impl<'a> Parser<'a> {
                 let unsafety = self.parse_unsafety();
 
                 if self.check_keyword(kw::Extern) {
-                    self.sess.gated_spans.const_extern_fn.borrow_mut().push(
-                        lo.to(self.token.span)
-                    );
+                    self.sess.gated_spans.gate(sym::const_extern_fn, lo.to(self.token.span));
                 }
                 let abi = self.parse_extern_abi()?;
                 self.bump(); // `fn`
@@ -831,7 +829,7 @@ impl<'a> Parser<'a> {
                     .emit();
             }
 
-            self.sess.gated_spans.trait_alias.borrow_mut().push(whole_span);
+            self.sess.gated_spans.gate(sym::trait_alias, whole_span);
 
             Ok((ident, ItemKind::TraitAlias(tps, bounds), None))
         } else {
@@ -1711,7 +1709,7 @@ impl<'a> Parser<'a> {
         let span = lo.to(self.prev_span);
 
         if !def.legacy {
-            self.sess.gated_spans.decl_macro.borrow_mut().push(span);
+            self.sess.gated_spans.gate(sym::decl_macro, span);
         }
 
         Ok(Some(self.mk_item(span, ident, ItemKind::MacroDef(def), vis.clone(), attrs.to_vec())))

src/libsyntax/parse/parser/pat.rs

@@ -8,9 +8,8 @@ use crate::mut_visit::{noop_visit_pat, noop_visit_mac, MutVisitor};
 use crate::parse::token::{self};
 use crate::print::pprust;
 use crate::source_map::{respan, Span, Spanned};
-use crate::symbol::kw;
 use crate::ThinVec;
-
+use syntax_pos::symbol::{kw, sym};
 use errors::{Applicability, DiagnosticBuilder};
 
 type Expected = Option<&'static str>;
@@ -52,11 +51,8 @@ impl<'a> Parser<'a> {
         // and no other gated or-pattern has been parsed thus far,
         // then we should really gate the leading `|`.
         // This complicated procedure is done purely for diagnostics UX.
-        if gated_leading_vert {
-            let mut or_pattern_spans = self.sess.gated_spans.or_patterns.borrow_mut();
-            if or_pattern_spans.is_empty() {
-                or_pattern_spans.push(leading_vert_span);
-            }
+        if gated_leading_vert && self.sess.gated_spans.is_ungated(sym::or_patterns) {
+            self.sess.gated_spans.gate(sym::or_patterns, leading_vert_span);
         }
 
         Ok(pat)
@@ -117,7 +113,7 @@ impl<'a> Parser<'a> {
 
         // Feature gate the or-pattern if instructed:
         if gate_or == GateOr::Yes {
-            self.sess.gated_spans.or_patterns.borrow_mut().push(or_pattern_span);
+            self.sess.gated_spans.gate(sym::or_patterns, or_pattern_span);
         }
 
         Ok(self.mk_pat(or_pattern_span, PatKind::Or(pats)))
@@ -325,7 +321,7 @@ impl<'a> Parser<'a> {
             } else if self.eat_keyword(kw::Box) {
                 // Parse `box pat`
                 let pat = self.parse_pat_with_range_pat(false, None)?;
-                self.sess.gated_spans.box_patterns.borrow_mut().push(lo.to(self.prev_span));
+                self.sess.gated_spans.gate(sym::box_patterns, lo.to(self.prev_span));
                 PatKind::Box(pat)
             } else if self.can_be_ident_pat() {
                 // Parse `ident @ pat`
@@ -612,7 +608,7 @@ impl<'a> Parser<'a> {
     }
 
     fn excluded_range_end(&self, span: Span) -> RangeEnd {
-        self.sess.gated_spans.exclusive_range_pattern.borrow_mut().push(span);
+        self.sess.gated_spans.gate(sym::exclusive_range_pattern, span);
         RangeEnd::Excluded
     }
 

src/libsyntax/parse/parser/path.rs

@@ -5,7 +5,7 @@ use crate::ast::{self, QSelf, Path, PathSegment, Ident, ParenthesizedArgs, Angle
 use crate::ast::{AnonConst, GenericArg, AssocTyConstraint, AssocTyConstraintKind, BlockCheckMode};
 use crate::parse::token::{self, Token};
 use crate::source_map::{Span, BytePos};
-use crate::symbol::kw;
+use syntax_pos::symbol::{kw, sym};
 
 use std::mem;
 use log::debug;
@@ -426,7 +426,7 @@ impl<'a> Parser<'a> {
 
                 // Gate associated type bounds, e.g., `Iterator<Item: Ord>`.
                 if let AssocTyConstraintKind::Bound { .. } = kind {
-                    self.sess.gated_spans.associated_type_bounds.borrow_mut().push(span);
+                    self.sess.gated_spans.gate(sym::associated_type_bounds, span);
                 }
 
                 constraints.push(AssocTyConstraint {

src/libsyntax/sess.rs

@@ -19,39 +19,53 @@ use std::str;
 /// Collected spans during parsing for places where a certain feature was
 /// used and should be feature gated accordingly in `check_crate`.
 #[derive(Default)]
-crate struct GatedSpans {
-    /// Spans collected for gating `let_chains`, e.g. `if a && let b = c {}`.
-    crate let_chains: Lock<Vec<Span>>,
-    /// Spans collected for gating `async_closure`, e.g. `async || ..`.
-    crate async_closure: Lock<Vec<Span>>,
-    /// Spans collected for gating `yield e?` expressions (`generators` gate).
-    crate yields: Lock<Vec<Span>>,
-    /// Spans collected for gating `or_patterns`, e.g. `Some(Foo | Bar)`.
-    crate or_patterns: Lock<Vec<Span>>,
-    /// Spans collected for gating `const_extern_fn`, e.g. `const extern fn foo`.
-    crate const_extern_fn: Lock<Vec<Span>>,
-    /// Spans collected for gating `trait_alias`, e.g. `trait Foo = Ord + Eq;`.
-    pub trait_alias: Lock<Vec<Span>>,
-    /// Spans collected for gating `associated_type_bounds`, e.g. `Iterator<Item: Ord>`.
-    pub associated_type_bounds: Lock<Vec<Span>>,
-    /// Spans collected for gating `crate_visibility_modifier`, e.g. `crate fn`.
-    pub crate_visibility_modifier: Lock<Vec<Span>>,
-    /// Spans collected for gating `const_generics`, e.g. `const N: usize`.
-    pub const_generics: Lock<Vec<Span>>,
-    /// Spans collected for gating `decl_macro`, e.g. `macro m() {}`.
-    pub decl_macro: Lock<Vec<Span>>,
-    /// Spans collected for gating `box_patterns`, e.g. `box 0`.
-    pub box_patterns: Lock<Vec<Span>>,
-    /// Spans collected for gating `exclusive_range_pattern`, e.g. `0..2`.
-    pub exclusive_range_pattern: Lock<Vec<Span>>,
-    /// Spans collected for gating `try_blocks`, e.g. `try { a? + b? }`.
-    pub try_blocks: Lock<Vec<Span>>,
-    /// Spans collected for gating `label_break_value`, e.g. `'label: { ... }`.
-    pub label_break_value: Lock<Vec<Span>>,
-    /// Spans collected for gating `box_syntax`, e.g. `box $expr`.
-    pub box_syntax: Lock<Vec<Span>>,
-    /// Spans collected for gating `type_ascription`, e.g. `42: usize`.
-    pub type_ascription: Lock<Vec<Span>>,
+pub struct GatedSpans {
+    pub spans: Lock<FxHashMap<Symbol, Vec<Span>>>,
+}
+
+impl GatedSpans {
+    /// Feature gate the given `span` under the given `feature`
+    /// which is same `Symbol` used in `active.rs`.
+    pub fn gate(&self, feature: Symbol, span: Span) {
+        self.spans
+            .borrow_mut()
+            .entry(feature)
+            .or_default()
+            .push(span);
+    }
+
+    /// Ungate the last span under the given `feature`.
+    /// Panics if the given `span` wasn't the last one.
+    ///
+    /// Using this is discouraged unless you have a really good reason to.
+    pub fn ungate_last(&self, feature: Symbol, span: Span) {
+        let removed_span = self.spans
+            .borrow_mut()
+            .entry(feature)
+            .or_default()
+            .pop()
+            .unwrap();
+        debug_assert_eq!(span, removed_span);
+    }
+
+    /// Is the provided `feature` gate ungated currently?
+    ///
+    /// Using this is discouraged unless you have a really good reason to.
+    pub fn is_ungated(&self, feature: Symbol) -> bool {
+        self.spans
+            .borrow()
+            .get(&feature)
+            .map_or(true, |spans| spans.is_empty())
+    }
+
+    /// Prepend the given set of `spans` onto the set in `self`.
+    pub fn merge(&self, mut spans: FxHashMap<Symbol, Vec<Span>>) {
+        let mut inner = self.spans.borrow_mut();
+        for (gate, mut gate_spans) in inner.drain() {
+            spans.entry(gate).or_default().append(&mut gate_spans);
+        }
+        *inner = spans;
+    }
 }
 
 /// Info about a parsing session.
@@ -72,7 +86,7 @@ pub struct ParseSess {
     /// analysis.
     pub ambiguous_block_expr_parse: Lock<FxHashMap<Span, Span>>,
     pub injected_crate_name: Once<Symbol>,
-    crate gated_spans: GatedSpans,
+    pub gated_spans: GatedSpans,
     /// The parser has reached `Eof` due to an unclosed brace. Used to silence unnecessary errors.
     pub reached_eof: Lock<bool>,
 }

src/libsyntax_expand/mbe/macro_rules.rs

@@ -29,7 +29,7 @@ use syntax_pos::Span;
 use rustc_data_structures::fx::FxHashMap;
 use std::borrow::Cow;
 use std::collections::hash_map::Entry;
-use std::slice;
+use std::{mem, slice};
 
 use errors::Applicability;
 use rustc_data_structures::sync::Lrc;
@@ -182,7 +182,6 @@ fn generic_extension<'cx>(
 
     // Which arm's failure should we report? (the one furthest along)
     let mut best_failure: Option<(Token, &str)> = None;
-
     for (i, lhs) in lhses.iter().enumerate() {
         // try each arm's matchers
         let lhs_tt = match *lhs {
@@ -190,8 +189,18 @@ fn generic_extension<'cx>(
             _ => cx.span_bug(sp, "malformed macro lhs"),
         };
 
+        // Take a snapshot of the state of pre-expansion gating at this point.
+        // This is used so that if a matcher is not `Success(..)`ful,
+        // then the spans which became gated when parsing the unsucessful matcher
+        // are not recorded. On the first `Success(..)`ful matcher, the spans are merged.
+        let mut gated_spans_snaphot = mem::take(&mut *cx.parse_sess.gated_spans.spans.borrow_mut());
+
         match parse_tt(cx, lhs_tt, arg.clone()) {
             Success(named_matches) => {
+                // The matcher was `Success(..)`ful.
+                // Merge the gated spans from parsing the matcher with the pre-existing ones.
+                cx.parse_sess.gated_spans.merge(gated_spans_snaphot);
+
                 let rhs = match rhses[i] {
                     // ignore delimiters
                     mbe::TokenTree::Delimited(_, ref delimed) => delimed.tts.clone(),
@@ -248,6 +257,10 @@ fn generic_extension<'cx>(
             },
             Error(err_sp, ref msg) => cx.span_fatal(err_sp.substitute_dummy(sp), &msg[..]),
         }
+
+        // The matcher was not `Success(..)`ful.
+        // Restore to the state before snapshotting and maybe try again.
+        mem::swap(&mut gated_spans_snaphot, &mut cx.parse_sess.gated_spans.spans.borrow_mut());
     }
 
     let (token, label) = best_failure.expect("ran no matchers");

src/test/ui/parser/issue-65846-rollback-gating-failing-matcher.rs

@@ -0,0 +1,14 @@
+// run-pass
+
+// Test that failing macro matchers will not cause pre-expansion errors
+// even though they use a feature that is pre-expansion gated.
+
+macro_rules! m {
+    ($e:expr) => { 0 }; // This fails on the input below due to `, foo`.
+    ($e:expr,) => { 1 }; // This also fails to match due to `foo`.
+    (box $e:expr, foo) => { 2 }; // Successful matcher, we should get `2`.
+}
+
+fn main() {
+    assert_eq!(2, m!(box 42, foo));
+}