-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
Don't try to recover keyword as non-keyword identifier #150590
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
Conversation
... in macro invocations. Issue: <rust-lang#149692>.
There's no sensible recovery scheme here, giving up the recovery is the right thing to do.
|
rustbot has assigned @petrochenkov. Use |
|
EDIT: hm, not very convinced it's worth it. |
|
@bors r+ |
Don't try to recover keyword as non-keyword identifier Fixes rust-lang#149692. On beta after rust-lang#146978, we ICE on ```rs macro_rules! m { ($id:item()) => {} } m!(Self()); ``` where `Self` in the macro invocation is a keyword not a "normal" identifier, while attempting to recover an missing keyword before an identifier. Except, `Self` *is* a keyword, so trying to parse that as a non-reserved identifier expectedly fails. I suspect rust-lang#146978 merely unmasked a possible code path to hit this case; this logic has been so for a good while. Previously, on stable, the error message looks something like ```rs error: expected identifier, found keyword `Self` --> src/lib.rs:5:4 | 5 | m!(Self()); | ^^^^ expected identifier, found keyword error: missing `fn` or `struct` for function or struct definition --> src/lib.rs:5:4 | 2 | ($id:item()) => {} | -------- while parsing argument for this `item` macro fragment ... 5 | m!(Self()); | ^^^^ | help: if you meant to call a macro, try | 5 | m!(Self!()); | + ``` I considered restoring this diagnostic, but I'm not super convinced it's worth the complexity (and to me, it's not super clear what the user actually intended here).
Don't try to recover keyword as non-keyword identifier Fixes rust-lang#149692. On beta after rust-lang#146978, we ICE on ```rs macro_rules! m { ($id:item()) => {} } m!(Self()); ``` where `Self` in the macro invocation is a keyword not a "normal" identifier, while attempting to recover an missing keyword before an identifier. Except, `Self` *is* a keyword, so trying to parse that as a non-reserved identifier expectedly fails. I suspect rust-lang#146978 merely unmasked a possible code path to hit this case; this logic has been so for a good while. Previously, on stable, the error message looks something like ```rs error: expected identifier, found keyword `Self` --> src/lib.rs:5:4 | 5 | m!(Self()); | ^^^^ expected identifier, found keyword error: missing `fn` or `struct` for function or struct definition --> src/lib.rs:5:4 | 2 | ($id:item()) => {} | -------- while parsing argument for this `item` macro fragment ... 5 | m!(Self()); | ^^^^ | help: if you meant to call a macro, try | 5 | m!(Self!()); | + ``` I considered restoring this diagnostic, but I'm not super convinced it's worth the complexity (and to me, it's not super clear what the user actually intended here).
…uwer Rollup of 12 pull requests Successful merges: - #150585 (Add a context-consistency check before emitting redundant generic-argument suggestions) - #150586 (rustdoc: Fix intra-doc link bugs involving type aliases and associated items) - #150590 (Don't try to recover keyword as non-keyword identifier ) - #150817 (cleanup: remove borrowck handling for inline const patterns) - #150939 (resolve: Relax some asserts in glob overwriting and add tests) - #150966 (rustc_target: Remove unused Arch::PowerPC64LE) - #150971 (Disallow eii in statement position) - #151016 (fix: WASI threading regression by disabling pthread usage) - #151046 (compiler: Make Externally Implementable Item (eii) macros "semiopaque") - #151103 (mir_build: Simplify length-determination and indexing for array/slice patterns) - #151117 (Avoid serde dependency in build_helper when not necessary) - #151127 (Delete `MetaItemOrLitParser::Err`) r? @ghost
Don't try to recover keyword as non-keyword identifier Fixes rust-lang#149692. On beta after rust-lang#146978, we ICE on ```rs macro_rules! m { ($id:item()) => {} } m!(Self()); ``` where `Self` in the macro invocation is a keyword not a "normal" identifier, while attempting to recover an missing keyword before an identifier. Except, `Self` *is* a keyword, so trying to parse that as a non-reserved identifier expectedly fails. I suspect rust-lang#146978 merely unmasked a possible code path to hit this case; this logic has been so for a good while. Previously, on stable, the error message looks something like ```rs error: expected identifier, found keyword `Self` --> src/lib.rs:5:4 | 5 | m!(Self()); | ^^^^ expected identifier, found keyword error: missing `fn` or `struct` for function or struct definition --> src/lib.rs:5:4 | 2 | ($id:item()) => {} | -------- while parsing argument for this `item` macro fragment ... 5 | m!(Self()); | ^^^^ | help: if you meant to call a macro, try | 5 | m!(Self!()); | + ``` I considered restoring this diagnostic, but I'm not super convinced it's worth the complexity (and to me, it's not super clear what the user actually intended here).
…uwer Rollup of 16 pull requests Successful merges: - #150585 (Add a context-consistency check before emitting redundant generic-argument suggestions) - #150586 (rustdoc: Fix intra-doc link bugs involving type aliases and associated items) - #150590 (Don't try to recover keyword as non-keyword identifier ) - #150817 (cleanup: remove borrowck handling for inline const patterns) - #150939 (resolve: Relax some asserts in glob overwriting and add tests) - #150962 (Remove `FeedConstTy` and provide ty when lowering const arg) - #150966 (rustc_target: Remove unused Arch::PowerPC64LE) - #150971 (Disallow eii in statement position) - #151016 (fix: WASI threading regression by disabling pthread usage) - #151046 (compiler: Make Externally Implementable Item (eii) macros "semiopaque") - #151099 (Recover parse gracefully from `<const N>`) - #151117 (Avoid serde dependency in build_helper when not necessary) - #151127 (Delete `MetaItemOrLitParser::Err`) - #151128 (Add temporary new bors e-mail address to the mailmap) - #151130 (resolve: Downgrade `ambiguous_glob_imports` to warn-by-default) - #151138 (Rename `rust.use-lld` to `rust.bootstrap-override-lld` in INSTALL.md) r? @ghost
…uwer Rollup of 15 pull requests Successful merges: - #150585 (Add a context-consistency check before emitting redundant generic-argument suggestions) - #150586 (rustdoc: Fix intra-doc link bugs involving type aliases and associated items) - #150590 (Don't try to recover keyword as non-keyword identifier ) - #150817 (cleanup: remove borrowck handling for inline const patterns) - #150939 (resolve: Relax some asserts in glob overwriting and add tests) - #150962 (Remove `FeedConstTy` and provide ty when lowering const arg) - #150966 (rustc_target: Remove unused Arch::PowerPC64LE) - #150971 (Disallow eii in statement position) - #151016 (fix: WASI threading regression by disabling pthread usage) - #151046 (compiler: Make Externally Implementable Item (eii) macros "semiopaque") - #151099 (Recover parse gracefully from `<const N>`) - #151117 (Avoid serde dependency in build_helper when not necessary) - #151127 (Delete `MetaItemOrLitParser::Err`) - #151128 (Add temporary new bors e-mail address to the mailmap) - #151138 (Rename `rust.use-lld` to `rust.bootstrap-override-lld` in INSTALL.md) r? @ghost
Rollup merge of #150590 - ident-kw-ice, r=petrochenkov Don't try to recover keyword as non-keyword identifier Fixes #149692. On beta after #146978, we ICE on ```rs macro_rules! m { ($id:item()) => {} } m!(Self()); ``` where `Self` in the macro invocation is a keyword not a "normal" identifier, while attempting to recover an missing keyword before an identifier. Except, `Self` *is* a keyword, so trying to parse that as a non-reserved identifier expectedly fails. I suspect #146978 merely unmasked a possible code path to hit this case; this logic has been so for a good while. Previously, on stable, the error message looks something like ```rs error: expected identifier, found keyword `Self` --> src/lib.rs:5:4 | 5 | m!(Self()); | ^^^^ expected identifier, found keyword error: missing `fn` or `struct` for function or struct definition --> src/lib.rs:5:4 | 2 | ($id:item()) => {} | -------- while parsing argument for this `item` macro fragment ... 5 | m!(Self()); | ^^^^ | help: if you meant to call a macro, try | 5 | m!(Self!()); | + ``` I considered restoring this diagnostic, but I'm not super convinced it's worth the complexity (and to me, it's not super clear what the user actually intended here).
…uwer Rollup of 15 pull requests Successful merges: - rust-lang/rust#150585 (Add a context-consistency check before emitting redundant generic-argument suggestions) - rust-lang/rust#150586 (rustdoc: Fix intra-doc link bugs involving type aliases and associated items) - rust-lang/rust#150590 (Don't try to recover keyword as non-keyword identifier ) - rust-lang/rust#150817 (cleanup: remove borrowck handling for inline const patterns) - rust-lang/rust#150939 (resolve: Relax some asserts in glob overwriting and add tests) - rust-lang/rust#150962 (Remove `FeedConstTy` and provide ty when lowering const arg) - rust-lang/rust#150966 (rustc_target: Remove unused Arch::PowerPC64LE) - rust-lang/rust#150971 (Disallow eii in statement position) - rust-lang/rust#151016 (fix: WASI threading regression by disabling pthread usage) - rust-lang/rust#151046 (compiler: Make Externally Implementable Item (eii) macros "semiopaque") - rust-lang/rust#151099 (Recover parse gracefully from `<const N>`) - rust-lang/rust#151117 (Avoid serde dependency in build_helper when not necessary) - rust-lang/rust#151127 (Delete `MetaItemOrLitParser::Err`) - rust-lang/rust#151128 (Add temporary new bors e-mail address to the mailmap) - rust-lang/rust#151138 (Rename `rust.use-lld` to `rust.bootstrap-override-lld` in INSTALL.md) r? @ghost
|
stable backport accepted during t-compiler triage, though we don't feel it deserve in isolation a dot release. (Backport in #151714) @rustbot label +stable-accepted |
…ckport-zulip-msg, r=jieyouxu update compiler stable backport zulip msg I'd like to update the message when stable backports (for t-compiler) Zulip topics are opened. Stable backports mention the channel i.e. also people who might not have context, example: [#t-compiler/backports > rust-lang#150590: stable-nominated @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23150590.3A.20stable-nominated/near/569989784) Beta backports mention author+reviewer (which fits better), example: [#t-compiler/backports > rust-lang#151896: beta-nominated @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23151896.3A.20beta-nominated/near/571171604) This patch makes the `stable` backport opening message just like the `beta` backport one. Thanks
…ckport-zulip-msg, r=jieyouxu update compiler stable backport zulip msg I'd like to update the message when stable backports (for t-compiler) Zulip topics are opened. Stable backports mention the channel i.e. also people who might not have context, example: [#t-compiler/backports > rust-lang#150590: stable-nominated @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23150590.3A.20stable-nominated/near/569989784) Beta backports mention author+reviewer (which fits better), example: [#t-compiler/backports > rust-lang#151896: beta-nominated @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23151896.3A.20beta-nominated/near/571171604) This patch makes the `stable` backport opening message just like the `beta` backport one. Thanks
…ckport-zulip-msg, r=jieyouxu update compiler stable backport zulip msg I'd like to update the message when stable backports (for t-compiler) Zulip topics are opened. Stable backports mention the channel i.e. also people who might not have context, example: [#t-compiler/backports > rust-lang#150590: stable-nominated @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23150590.3A.20stable-nominated/near/569989784) Beta backports mention author+reviewer (which fits better), example: [#t-compiler/backports > rust-lang#151896: beta-nominated @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23151896.3A.20beta-nominated/near/571171604) This patch makes the `stable` backport opening message just like the `beta` backport one. Thanks
Rollup merge of #152182 - apiraino:update-compiler-stable-backport-zulip-msg, r=jieyouxu update compiler stable backport zulip msg I'd like to update the message when stable backports (for t-compiler) Zulip topics are opened. Stable backports mention the channel i.e. also people who might not have context, example: [#t-compiler/backports > #150590: stable-nominated @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23150590.3A.20stable-nominated/near/569989784) Beta backports mention author+reviewer (which fits better), example: [#t-compiler/backports > #151896: beta-nominated @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/474880-t-compiler.2Fbackports/topic/.23151896.3A.20beta-nominated/near/571171604) This patch makes the `stable` backport opening message just like the `beta` backport one. Thanks
Fixes #149692.
On beta after #146978, we ICE on
where
Selfin the macro invocation is a keyword not a "normal" identifier, while attempting to recover an missing keyword before an identifier. Except,Selfis a keyword, so trying to parse that as a non-reserved identifier expectedly fails.I suspect #146978 merely unmasked a possible code path to hit this case; this logic has been so for a good while. Previously, on stable, the error message looks something like
I considered restoring this diagnostic, but I'm not super convinced it's worth the complexity (and to me, it's not super clear what the user actually intended here).