Skip to content

Commit

Permalink
Unrolled build for rust-lang#126762
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#126762 - compiler-errors:kw-lt, r=michaelwoerister

Deny keyword lifetimes pre-expansion

rust-lang#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: rust-lang#126452 (comment)

This probably should get lang FCP'd just for consistency.
  • Loading branch information
rust-timer committed Jul 17, 2024
2 parents 032be6f + d0a1851 commit d748cbf
Show file tree
Hide file tree
Showing 14 changed files with 99 additions and 70 deletions.
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 @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) => {
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

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

0 comments on commit d748cbf

Please sign in to comment.