about summary refs log tree commit diff
diff options
context:
space:
mode:
authorTrevor Gross <t.gross35@gmail.com>2024-07-16 16:15:15 -0500
committerGitHub <noreply@github.com>2024-07-16 16:15:15 -0500
commit9833e21c5db09ec666f044340f3fc71dd5742216 (patch)
tree5fd26e4fe54086475cbf147eb911ee02b0da47e5
parent36ea06827bf32cb4d12bf479e5a56c54bd358611 (diff)
parentd0a1851ec2cf84bdb41bc4d788b3995a8463c543 (diff)
downloadrust-9833e21c5db09ec666f044340f3fc71dd5742216.tar.gz
rust-9833e21c5db09ec666f044340f3fc71dd5742216.zip
Rollup merge of #126762 - compiler-errors:kw-lt, r=michaelwoerister
Deny keyword lifetimes pre-expansion

https://github.com/rust-lang/rust/pull/126452#issuecomment-2179464266

> 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: https://github.com/rust-lang/rust/pull/126452#issuecomment-2171654756

This probably should get lang FCP'd just for consistency.
-rw-r--r--compiler/rustc_ast_passes/messages.ftl6
-rw-r--r--compiler/rustc_ast_passes/src/ast_validation.rs30
-rw-r--r--compiler/rustc_ast_passes/src/errors.rs15
-rw-r--r--compiler/rustc_parse/messages.ftl6
-rw-r--r--compiler/rustc_parse/src/errors.rs15
-rw-r--r--compiler/rustc_parse/src/parser/expr.rs13
-rw-r--r--compiler/rustc_parse/src/parser/nonterminal.rs7
-rw-r--r--compiler/rustc_parse/src/parser/pat.rs8
-rw-r--r--compiler/rustc_parse/src/parser/ty.rs6
-rw-r--r--tests/ui/parser/cfg-keyword-lifetime.rs15
-rw-r--r--tests/ui/parser/cfg-keyword-lifetime.stderr14
-rw-r--r--tests/ui/parser/require-parens-for-chained-comparison.rs2
-rw-r--r--tests/ui/parser/require-parens-for-chained-comparison.stderr20
-rw-r--r--tests/ui/self/self_type_keyword.stderr12
14 files changed, 99 insertions, 70 deletions
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 e6cc2e4069b..83249dea82a 100644
--- a/compiler/rustc_ast_passes/src/ast_validation.rs
+++ b/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) => {
diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs
index 2c18b47f0f7..460da254653 100644
--- a/compiler/rustc_ast_passes/src/errors.rs
+++ b/compiler/rustc_ast_passes/src/errors.rs
@@ -10,21 +10,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 {
     #[primary_span]
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
@@ -2010,6 +2010,21 @@ pub struct CannotBeRawIdent {
 }
 
 #[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 {
     #[primary_span]
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 @@ 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).
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 @@ 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,
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 @@ 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),
             });
         }
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 @@ 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 {
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
    |