Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deny keyword lifetimes pre-expansion #126762

Merged
merged 1 commit into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions compiler/rustc_ast_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
30 changes: 0 additions & 30 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,19 +284,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;
Expand Down Expand Up @@ -923,16 +910,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)
Expand Down Expand Up @@ -1371,13 +1348,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) => {
Expand Down
15 changes: 0 additions & 15 deletions compiler/rustc_ast_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 10 additions & 3 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_parse/src/parser/nonterminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
});
}
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/parser/cfg-keyword-lifetime.rs
Original file line number Diff line number Diff line change
@@ -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() {}
14 changes: 14 additions & 0 deletions tests/ui/parser/cfg-keyword-lifetime.stderr
Original file line number Diff line number Diff line change
@@ -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

2 changes: 2 additions & 0 deletions tests/ui/parser/require-parens-for-chained-comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it slightly unexpected that the error is about labels instead of about lifetimes. I guess this is because it is not even trying to parse this as a path at this point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is being parsed as "f is less than..."


let _ = f<u8>;
//~^ ERROR comparison operators cannot be chained
Expand Down
20 changes: 16 additions & 4 deletions tests/ui/parser/require-parens-for-chained-comparison.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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
Expand All @@ -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<'_>();
| ^ ^
Expand All @@ -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

12 changes: 6 additions & 6 deletions tests/ui/self/self_type_keyword.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down Expand Up @@ -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
|
Expand Down
Loading