Rollup merge of #126762 - compiler-errors:kw-lt, r=michaelwoerister

Deny keyword lifetimes pre-expansion

#126452 (comment)

> Secondly, we confirmed that we're OK with moving the validation of keywords in lifetimes to pre-expansion from post-expansion. We similarly consider this a bug fix. While the breakage of the convenience feature of the with_locals crate that relies on this is unfortunate, and we wish we had not overlooked this earlier for that reason, we're fortunate that the breakage is contained to only one crate, and we're going to accept this breakage as the extra complexity we'd need to carry in the compiler to work around this isn't deemed worth it.

T-lang considers it to be a bugfix to deny `'keyword` lifetimes in the parser, rather than during AST validation that only happens post-expansion. This has one breakage: #126452 (comment)

This probably should get lang FCP'd just for consistency.

Author: tgross35

compiler/rustc_ast_passes/messages.ftl

@@ -159,9 +159,6 @@ ast_passes_inherent_cannot_be = inherent impls cannot be {$annotation}
     .type = inherent impl for this type
     .only_trait = only trait implementations may be annotated with {$annotation}
 
-ast_passes_invalid_label =
-    invalid label name `{$name}`
-
 ast_passes_invalid_unnamed_field =
     unnamed fields are not allowed outside of structs or unions
     .label = unnamed field declared here
@@ -176,9 +173,6 @@ ast_passes_item_invalid_safety = items outside of `unsafe extern {"{ }"}` cannot
 ast_passes_item_underscore = `{$kind}` items in this context need a name
     .label = `_` is not a valid name for this `{$kind}` item
 
-ast_passes_keyword_lifetime =
-    lifetimes cannot use keyword names
-
 ast_passes_match_arm_with_no_body =
     `match` arm with no body
     .suggestion = add a body after the pattern

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -269,19 +269,6 @@ impl<'a> AstValidator<'a> {
         self.session.dcx()
     }
 
-    fn check_lifetime(&self, ident: Ident) {
-        let valid_names = [kw::UnderscoreLifetime, kw::StaticLifetime, kw::Empty];
-        if !valid_names.contains(&ident.name) && ident.without_first_quote().is_reserved() {
-            self.dcx().emit_err(errors::KeywordLifetime { span: ident.span });
-        }
-    }
-
-    fn check_label(&self, ident: Ident) {
-        if ident.without_first_quote().is_reserved() {
-            self.dcx().emit_err(errors::InvalidLabel { span: ident.span, name: ident.name });
-        }
-    }
-
     fn visibility_not_permitted(&self, vis: &Visibility, note: errors::VisibilityNotPermittedNote) {
         if let VisibilityKind::Inherited = vis.kind {
             return;
@@ -908,16 +895,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         self.walk_ty(ty)
     }
 
-    fn visit_label(&mut self, label: &'a Label) {
-        self.check_label(label.ident);
-        visit::walk_label(self, label);
-    }
-
-    fn visit_lifetime(&mut self, lifetime: &'a Lifetime, _: visit::LifetimeCtxt) {
-        self.check_lifetime(lifetime.ident);
-        visit::walk_lifetime(self, lifetime);
-    }
-
     fn visit_field_def(&mut self, field: &'a FieldDef) {
         self.deny_unnamed_field(field);
         visit::walk_field_def(self, field)
@@ -1356,13 +1333,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         }
     }
 
-    fn visit_generic_param(&mut self, param: &'a GenericParam) {
-        if let GenericParamKind::Lifetime { .. } = param.kind {
-            self.check_lifetime(param.ident);
-        }
-        visit::walk_generic_param(self, param);
-    }
-
     fn visit_param_bound(&mut self, bound: &'a GenericBound, ctxt: BoundKind) {
         match bound {
             GenericBound::Trait(trait_ref, modifiers) => {

compiler/rustc_ast_passes/src/errors.rs

@@ -9,21 +9,6 @@ use rustc_span::{symbol::Ident, Span, Symbol};
 
 use crate::fluent_generated as fluent;
 
-#[derive(Diagnostic)]
-#[diag(ast_passes_keyword_lifetime)]
-pub struct KeywordLifetime {
-    #[primary_span]
-    pub span: Span,
-}
-
-#[derive(Diagnostic)]
-#[diag(ast_passes_invalid_label)]
-pub struct InvalidLabel {
-    #[primary_span]
-    pub span: Span,
-    pub name: Symbol,
-}
-
 #[derive(Diagnostic)]
 #[diag(ast_passes_visibility_not_permitted, code = E0449)]
 pub struct VisibilityNotPermitted {

compiler/rustc_parse/messages.ftl

@@ -388,6 +388,9 @@ parse_invalid_dyn_keyword = invalid `dyn` keyword
 parse_invalid_expression_in_let_else = a `{$operator}` expression cannot be directly assigned in `let...else`
 parse_invalid_identifier_with_leading_number = identifiers cannot start with a number
 
+parse_invalid_label =
+    invalid label name `{$name}`
+
 parse_invalid_literal_suffix_on_tuple_index = suffixes on a tuple index are invalid
     .label = invalid suffix `{$suffix}`
     .tuple_exception_line_1 = `{$suffix}` is *temporarily* accepted on tuple index fields as it was incorrectly accepted on stable for a few releases
@@ -414,6 +417,9 @@ parse_invalid_unicode_escape = invalid unicode character escape
 parse_invalid_variable_declaration =
     invalid variable declaration
 
+parse_keyword_lifetime =
+    lifetimes cannot use keyword names
+
 parse_kw_bad_case = keyword `{$kw}` is written in the wrong case
     .suggestion = write it in the correct case
 

compiler/rustc_parse/src/errors.rs

@@ -2009,6 +2009,21 @@ pub struct CannotBeRawIdent {
     pub ident: Symbol,
 }
 
+#[derive(Diagnostic)]
+#[diag(parse_keyword_lifetime)]
+pub struct KeywordLifetime {
+    #[primary_span]
+    pub span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(parse_invalid_label)]
+pub struct InvalidLabel {
+    #[primary_span]
+    pub span: Span,
+    pub name: Symbol,
+}
+
 #[derive(Diagnostic)]
 #[diag(parse_cr_doc_comment)]
 pub struct CrDocComment {

compiler/rustc_parse/src/parser/expr.rs

@@ -2932,10 +2932,17 @@ impl<'a> Parser<'a> {
     }
 
     pub(crate) fn eat_label(&mut self) -> Option<Label> {
-        self.token.lifetime().map(|ident| {
+        if let Some(ident) = self.token.lifetime() {
+            // Disallow `'fn`, but with a better error message than `expect_lifetime`.
+            if ident.without_first_quote().is_reserved() {
+                self.dcx().emit_err(errors::InvalidLabel { span: ident.span, name: ident.name });
+            }
+
             self.bump();
-            Label { ident }
-        })
+            Some(Label { ident })
+        } else {
+            None
+        }
     }
 
     /// Parses a `match ... { ... }` expression (`match` token already eaten).

compiler/rustc_parse/src/parser/nonterminal.rs

@@ -177,8 +177,11 @@ impl<'a> Parser<'a> {
                     .collect_tokens_no_attrs(|this| this.parse_visibility(FollowedByType::Yes))?))
             }
             NonterminalKind::Lifetime => {
-                return if self.check_lifetime() {
-                    Ok(ParseNtResult::Lifetime(self.expect_lifetime().ident))
+                // We want to keep `'keyword` parsing, just like `keyword` is still
+                // an ident for nonterminal purposes.
+                return if let Some(ident) = self.token.lifetime() {
+                    self.bump();
+                    Ok(ParseNtResult::Lifetime(ident))
                 } else {
                     Err(self.dcx().create_err(UnexpectedNonterminal::Lifetime {
                         span: self.token.span,

compiler/rustc_parse/src/parser/pat.rs

@@ -542,12 +542,12 @@ impl<'a> Parser<'a> {
                     None => PatKind::Path(qself, path),
                 }
             }
-        } else if let token::Lifetime(lt) = self.token.kind
+        } else if let Some(lt) = self.token.lifetime()
             // In pattern position, we're totally fine with using "next token isn't colon"
             // as a heuristic. We could probably just always try to recover if it's a lifetime,
             // because we never have `'a: label {}` in a pattern position anyways, but it does
             // keep us from suggesting something like `let 'a: Ty = ..` => `let 'a': Ty = ..`
-            && could_be_unclosed_char_literal(Ident::with_dummy_span(lt))
+            && could_be_unclosed_char_literal(lt)
             && !self.look_ahead(1, |token| matches!(token.kind, token::Colon))
         {
             // Recover a `'a` as a `'a'` literal
@@ -683,12 +683,12 @@ impl<'a> Parser<'a> {
     /// Parse `&pat` / `&mut pat`.
     fn parse_pat_deref(&mut self, expected: Option<Expected>) -> PResult<'a, PatKind> {
         self.expect_and()?;
-        if let token::Lifetime(name) = self.token.kind {
+        if let Some(lifetime) = self.token.lifetime() {
             self.bump(); // `'a`
 
             self.dcx().emit_err(UnexpectedLifetimeInPattern {
                 span: self.prev_token.span,
-                symbol: name,
+                symbol: lifetime.name,
                 suggestion: self.prev_token.span.until(self.token.span),
             });
         }

compiler/rustc_parse/src/parser/ty.rs

@@ -1230,6 +1230,12 @@ impl<'a> Parser<'a> {
     /// Parses a single lifetime `'a` or panics.
     pub(super) fn expect_lifetime(&mut self) -> Lifetime {
         if let Some(ident) = self.token.lifetime() {
+            if ident.without_first_quote().is_reserved()
+                && ![kw::UnderscoreLifetime, kw::StaticLifetime].contains(&ident.name)
+            {
+                self.dcx().emit_err(errors::KeywordLifetime { span: ident.span });
+            }
+
             self.bump();
             Lifetime { ident, id: ast::DUMMY_NODE_ID }
         } else {

tests/ui/parser/cfg-keyword-lifetime.rs

@@ -0,0 +1,15 @@
+// Disallow `'keyword` even in cfg'd code.
+
+#[cfg(any())]
+fn hello() -> &'ref () {}
+//~^ ERROR lifetimes cannot use keyword names
+
+macro_rules! macro_invocation {
+    ($i:item) => {}
+}
+macro_invocation! {
+    fn hello() -> &'ref () {}
+    //~^ ERROR lifetimes cannot use keyword names
+}
+
+fn main() {}

tests/ui/parser/cfg-keyword-lifetime.stderr

@@ -0,0 +1,14 @@
+error: lifetimes cannot use keyword names
+  --> $DIR/cfg-keyword-lifetime.rs:4:16
+   |
+LL | fn hello() -> &'ref () {}
+   |                ^^^^
+
+error: lifetimes cannot use keyword names
+  --> $DIR/cfg-keyword-lifetime.rs:11:20
+   |
+LL |     fn hello() -> &'ref () {}
+   |                    ^^^^
+
+error: aborting due to 2 previous errors
+

tests/ui/parser/require-parens-for-chained-comparison.rs

@@ -24,12 +24,14 @@ fn main() {
     //~| HELP use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
     //~| ERROR expected
     //~| HELP add `'` to close the char literal
+    //~| ERROR invalid label name
 
     f<'_>();
     //~^ comparison operators cannot be chained
     //~| HELP use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
     //~| ERROR expected
     //~| HELP add `'` to close the char literal
+    //~| ERROR invalid label name
 
     let _ = f<u8>;
     //~^ ERROR comparison operators cannot be chained

tests/ui/parser/require-parens-for-chained-comparison.stderr

@@ -53,6 +53,12 @@ help: use `::<...>` instead of `<...>` to specify lifetime, type, or const argum
 LL |     let _ = f::<u8, i8>();
    |              ++
 
+error: invalid label name `'_`
+  --> $DIR/require-parens-for-chained-comparison.rs:22:15
+   |
+LL |     let _ = f<'_, i8>();
+   |               ^^
+
 error: expected `while`, `for`, `loop` or `{` after a label
   --> $DIR/require-parens-for-chained-comparison.rs:22:17
    |
@@ -75,8 +81,14 @@ help: use `::<...>` instead of `<...>` to specify lifetime, type, or const argum
 LL |     let _ = f::<'_, i8>();
    |              ++
 
+error: invalid label name `'_`
+  --> $DIR/require-parens-for-chained-comparison.rs:29:7
+   |
+LL |     f<'_>();
+   |       ^^
+
 error: expected `while`, `for`, `loop` or `{` after a label
-  --> $DIR/require-parens-for-chained-comparison.rs:28:9
+  --> $DIR/require-parens-for-chained-comparison.rs:29:9
    |
 LL |     f<'_>();
    |         ^ expected `while`, `for`, `loop` or `{` after a label
@@ -87,7 +99,7 @@ LL |     f<'_'>();
    |         +
 
 error: comparison operators cannot be chained
-  --> $DIR/require-parens-for-chained-comparison.rs:28:6
+  --> $DIR/require-parens-for-chained-comparison.rs:29:6
    |
 LL |     f<'_>();
    |      ^  ^
@@ -98,13 +110,13 @@ LL |     f::<'_>();
    |      ++
 
 error: comparison operators cannot be chained
-  --> $DIR/require-parens-for-chained-comparison.rs:34:14
+  --> $DIR/require-parens-for-chained-comparison.rs:36:14
    |
 LL |     let _ = f<u8>;
    |              ^  ^
    |
    = help: use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
    = help: or use `(...)` if you meant to specify fn arguments
 
-error: aborting due to 10 previous errors
+error: aborting due to 12 previous errors
 

tests/ui/self/self_type_keyword.stderr

@@ -4,6 +4,12 @@ error: expected identifier, found keyword `Self`
 LL |   struct Self;
    |          ^^^^ expected identifier, found keyword
 
+error: lifetimes cannot use keyword names
+  --> $DIR/self_type_keyword.rs:6:12
+   |
+LL | struct Bar<'Self>;
+   |            ^^^^^
+
 error: expected identifier, found keyword `Self`
   --> $DIR/self_type_keyword.rs:14:13
    |
@@ -53,12 +59,6 @@ error: expected identifier, found keyword `Self`
 LL |     trait Self {}
    |           ^^^^ expected identifier, found keyword
 
-error: lifetimes cannot use keyword names
-  --> $DIR/self_type_keyword.rs:6:12
-   |
-LL | struct Bar<'Self>;
-   |            ^^^^^
-
 error: cannot find macro `Self` in this scope
   --> $DIR/self_type_keyword.rs:21:9
    |