From d0a1851ec2cf84bdb41bc4d788b3995a8463c543 Mon Sep 17 00:00:00 2001
From: Michael Goulet <michael@errs.io>
Date: Thu, 20 Jun 2024 16:36:35 -0400
Subject: [PATCH] Deny keyword lifetimes pre-expansion

---
 compiler/rustc_ast_passes/messages.ftl        |  6 ----
 .../rustc_ast_passes/src/ast_validation.rs    | 30 -------------------
 compiler/rustc_ast_passes/src/errors.rs       | 15 ----------
 compiler/rustc_parse/messages.ftl             |  6 ++++
 compiler/rustc_parse/src/errors.rs            | 15 ++++++++++
 compiler/rustc_parse/src/parser/expr.rs       | 13 ++++++--
 .../rustc_parse/src/parser/nonterminal.rs     |  7 +++--
 compiler/rustc_parse/src/parser/pat.rs        |  8 ++---
 compiler/rustc_parse/src/parser/ty.rs         |  6 ++++
 tests/ui/parser/cfg-keyword-lifetime.rs       | 15 ++++++++++
 tests/ui/parser/cfg-keyword-lifetime.stderr   | 14 +++++++++
 .../require-parens-for-chained-comparison.rs  |  2 ++
 ...quire-parens-for-chained-comparison.stderr | 20 ++++++++++---
 tests/ui/self/self_type_keyword.stderr        | 12 ++++----
 14 files changed, 99 insertions(+), 70 deletions(-)
 create mode 100644 tests/ui/parser/cfg-keyword-lifetime.rs
 create mode 100644 tests/ui/parser/cfg-keyword-lifetime.stderr

diff --git a/compiler/rustc_ast_passes/messages.ftl b/compiler/rustc_ast_passes/messages.ftl
index 7da726ef408..02bdff96aa6 100644
--- a/compiler/rustc_ast_passes/messages.ftl
+++ b/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
diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs
index dd0d904c52c..f33176c0b32 100644
--- a/compiler/rustc_ast_passes/src/ast_validation.rs
+++ b/compiler/rustc_ast_passes/src/ast_validation.rs
@@ -284,19 +284,6 @@ fn dcx(&self) -> DiagCtxtHandle<'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;
@@ -923,16 +910,6 @@ fn visit_ty(&mut self, ty: &'a Ty) {
         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)
@@ -1371,13 +1348,6 @@ fn visit_generics(&mut self, generics: &'a Generics) {
         }
     }
 
-    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) => {
diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs
index bfb90476450..4dff4ecfae0 100644
--- a/compiler/rustc_ast_passes/src/errors.rs
+++ b/compiler/rustc_ast_passes/src/errors.rs
@@ -9,21 +9,6 @@
 
 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 {
diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl
index c2201b1c41e..4ce9e0f025c 100644
--- a/compiler/rustc_parse/messages.ftl
+++ b/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
 
diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs
index 092a2a10ab7..4222486034b 100644
--- a/compiler/rustc_parse/src/errors.rs
+++ b/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 {
diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
index 4bd20be4171..0ba8c66f48f 100644
--- a/compiler/rustc_parse/src/parser/expr.rs
+++ b/compiler/rustc_parse/src/parser/expr.rs
@@ -2932,10 +2932,17 @@ fn parse_expr_loop(&mut self, opt_label: Option<Label>, lo: Span) -> PResult<'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).
diff --git a/compiler/rustc_parse/src/parser/nonterminal.rs b/compiler/rustc_parse/src/parser/nonterminal.rs
index 4a78b427832..41e31d76d62 100644
--- a/compiler/rustc_parse/src/parser/nonterminal.rs
+++ b/compiler/rustc_parse/src/parser/nonterminal.rs
@@ -177,8 +177,11 @@ pub fn parse_nonterminal(&mut self, kind: NonterminalKind) -> PResult<'a, ParseN
                     .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,
diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs
index e4e89615d71..8e8df9f0a84 100644
--- a/compiler/rustc_parse/src/parser/pat.rs
+++ b/compiler/rustc_parse/src/parser/pat.rs
@@ -542,12 +542,12 @@ fn parse_pat_with_range_pat(
                     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 @@ fn ban_pat_range_if_ambiguous(&self, pat: &Pat) {
     /// 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),
             });
         }
diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs
index 94321b1dddd..68b8af7d20e 100644
--- a/compiler/rustc_parse/src/parser/ty.rs
+++ b/compiler/rustc_parse/src/parser/ty.rs
@@ -1230,6 +1230,12 @@ pub(super) fn check_lifetime(&mut self) -> bool {
     /// 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 {
diff --git a/tests/ui/parser/cfg-keyword-lifetime.rs b/tests/ui/parser/cfg-keyword-lifetime.rs
new file mode 100644
index 00000000000..a1588eddc07
--- /dev/null
+++ b/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() {}
diff --git a/tests/ui/parser/cfg-keyword-lifetime.stderr b/tests/ui/parser/cfg-keyword-lifetime.stderr
new file mode 100644
index 00000000000..52d305e2521
--- /dev/null
+++ b/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
+
diff --git a/tests/ui/parser/require-parens-for-chained-comparison.rs b/tests/ui/parser/require-parens-for-chained-comparison.rs
index 5b90e905a64..916f1b83db2 100644
--- a/tests/ui/parser/require-parens-for-chained-comparison.rs
+++ b/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
diff --git a/tests/ui/parser/require-parens-for-chained-comparison.stderr b/tests/ui/parser/require-parens-for-chained-comparison.stderr
index 52e201c435c..857c4a55788 100644
--- a/tests/ui/parser/require-parens-for-chained-comparison.stderr
+++ b/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,7 +110,7 @@ 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>;
    |              ^  ^
@@ -106,5 +118,5 @@ 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
 
diff --git a/tests/ui/self/self_type_keyword.stderr b/tests/ui/self/self_type_keyword.stderr
index 8298293a8cb..f9cde810cad 100644
--- a/tests/ui/self/self_type_keyword.stderr
+++ b/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
    |