Lex lifetimes as identifiers, recover from emoji and emit appropriate error#153893
Lex lifetimes as identifiers, recover from emoji and emit appropriate error#153893estebank wants to merge 6 commits intorust-lang:mainfrom
Conversation
|
The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease |
|
rustbot has assigned @jdonszelmann. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
8679628 to
cf3c735
Compare
This comment has been minimized.
This comment has been minimized.
compiler/rustc_lexer/src/lib.rs
Outdated
| self.bump(); | ||
| self.eat_while(is_id_continue); | ||
| self.eat_while(|c| { | ||
| let is_emoji = !c.is_ascii() && c.is_emoji_char(); |
There was a problem hiding this comment.
can we possibly create a function or a method for this, i see this logic is using in two places, maybe something like self.is_emoji() would be better approach?
| if has_emoji { | ||
| self.dcx().struct_span_err(span, "lifetimes cannot contain emoji").emit(); | ||
| } |
There was a problem hiding this comment.
Why not make use of the preexisting ParseSess.bad_unicode_identifiers & rustc_interface::passes infrastructure?
There was a problem hiding this comment.
There was a problem hiding this comment.
The main reason to not do that is to still leverage the 'blah' recovery machinery. Here we are in a situation where we have to either choose one case to have nicer output than the other, or find a good way to unify both checks.
Lex and parse emoji in lifetimes, and disallow them in the parser with a hard error. Allow emoji to start a lifetime name even if they are not XID_Start. ``` error: lifetimes cannot contain emoji --> $DIR/emoji-in-lifetime.rs:1:22 | LL | fn bad_lifetime_name<'🐛🐛🐛family👨👩👧👦>( | ^^^^^^^^^^^^^^^^^^^^^ ```
cf3c735 to
a45ca0d
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
a45ca0d to
dfdc525
Compare
This comment has been minimized.
This comment has been minimized.
|
rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead. cc @rust-lang/rust-analyzer |
This comment has been minimized.
This comment has been minimized.
|
I agree with esteban for the nicer suggestions here, that's probably worth it. Sorry it took me a while, but lgtm. @bors r=me,veykril rollup |
|
@bors r=jdonszelmann,Veykril |
…lmann,Veykril Lex lifetimes as identifiers, recover from emoji and emit appropriate error Lex and parse emoji in lifetimes by using the identifier logic, and disallow them in the parser with a hard error. Allow emoji to start a lifetime name even if they are not XID_Start. ``` error: identifiers cannot contain emoji: `'🐛🐛🐛family👨👩👧👦` --> $DIR/emoji-in-lifetime.rs:1:22 | LL | fn bad_lifetime_name<'🐛🐛🐛family👨👩👧👦>( | ^^^^^^^^^^^^^^^^^^^^^ ``` Address rust-lang#141081 (but we could provide more information in the diagnostic, pointing at the specific chars, providing a link to the reference on identifiers and/or some other extra information).
Rollup of 7 pull requests Successful merges: - #142659 (compiler-builtins: Clean up features) - #153574 (Avoid ICE when param-env normalization leaves unresolved inference variables) - #153648 (Fix EII function aliases eliminated by LTO) - #153790 (Fix regression when dealing with generics/values with unresolved inference) - #153893 (Lex lifetimes as identifiers, recover from emoji and emit appropriate error) - #153980 (refactor: move doc(rust_logo) check to parser) - #154551 (Skip suggestions pointing to macro def for assert_eq)
Rollup of 7 pull requests Successful merges: - #142659 (compiler-builtins: Clean up features) - #153574 (Avoid ICE when param-env normalization leaves unresolved inference variables) - #153648 (Fix EII function aliases eliminated by LTO) - #153790 (Fix regression when dealing with generics/values with unresolved inference) - #153893 (Lex lifetimes as identifiers, recover from emoji and emit appropriate error) - #153980 (refactor: move doc(rust_logo) check to parser) - #154551 (Skip suggestions pointing to macro def for assert_eq)
|
Failed in rollup: #154611 (comment) |
|
This pull request was unapproved. This PR was contained in a rollup (#154611), which was unapproved. |
This comment has been minimized.
This comment has been minimized.
Lex lifetimes as identifiers, recover from emoji and emit appropriate error try-job: dist-ohos-armv7
|
💔 Test for 8db3bb9 failed: CI. Failed job:
|
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Lex and parse emoji in lifetimes by using the identifier logic, and disallow them in the parser with a hard error. Allow emoji to start a lifetime name even if they are not XID_Start.
Address #141081 (but we could provide more information in the diagnostic, pointing at the specific chars, providing a link to the reference on identifiers and/or some other extra information).