compiler: suggest `const _` for a misplaced `const {}` #128374

workingjubilee · 2024-07-29T23:38:42Z

rustbot · 2024-07-29T23:38:49Z

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

compiler/rustc_parse/src/parser/item.rs

tests/ui/inline-const/expr-in-item-context.stderr

compiler-errors · 2024-07-29T23:49:16Z

compiler/rustc_parse/src/parser/item.rs

@@ -81,6 +81,11 @@ impl<'a> Parser<'a> {
                        span,
                        "consider using `const` or `static` instead of `let` for global variables",
                    );
+                } else if self.parse_const_block(span, false).is_ok() {


Using is_ok here is kinda hazardous since you need to cancel the diagnostic on the bad path.

that is... definitely the part I was least certain about, yes. is there, like, a sample of Rust that will now elicit some disastrously bad diagnostic as a result of doing this, or...?

No, if you drop a Diagnostic without emitting it the compiler will ICE.

Presumably you will have some code that looks like:

match self.parse_const_block(span, false) { Err(e) => { e.cancel(); } Ok(_) => { do the recovery code here } }

Which at that point we probably should do snapshotting too, so try this:

let snapshot = self.create_snapshot_for_diagnostic(); match self.parse_const_block(span, false) { Err(e) => { e.cancel(); self.recover_from_snapshot(snapshot); } Ok(_) => { do the recovery code here } }

No, if you drop a Diagnostic without emitting it the compiler will ICE.

...oh lmao, drop bombs away.

Example of the drop bomb in question -- this ICEs after your PR:

const { a: () = 1; };

compiler-errors · 2024-07-29T23:49:51Z

compiler/rustc_parse/src/parser/item.rs

@@ -81,6 +81,11 @@ impl<'a> Parser<'a> {
                        span,
                        "consider using `const` or `static` instead of `let` for global variables",
                    );
+                } else if self.parse_const_block(span, false).is_ok() {


This logic should probably be moved to parse_item (well, one of its helpers like parse_item_common) since this is really an item recovery.

should I just yeet all these suggestions that are secretly item recoveries over there?

If you'd like to do so, please do.

yeah, I think it's important that both of these suggestions continue to live next to each other so they get refactored together.

compiler-errors · 2024-07-29T23:52:15Z

compiler/rustc_parse/src/parser/item.rs

+                    err.span_label(
+                        span,


Also please use a structured suggestion (with Applicability::MaybeIncorrect)

err.span_suggestion_verbose( span.shrink_to_lo(), /* ... */ )

The suggestion above this one for the kw::Let case should also be using something like that, right?

Yea, though that one's harder because it has two choices. You can try to use span_suggestions if you'd like.

workingjubilee · 2024-07-30T00:10:20Z

oh THERE'S the broken tests lmao

compiler-errors · 2024-07-30T00:12:23Z

@workingjubilee:

You should probably also check that the current token is kw::Const before proceeding to call parse_const. You may also want to check that the self.lookahead(1, ..)'th token is {.

workingjubilee · 2024-07-30T00:47:32Z

fun times ahead,

@rustbot author

Co-authored-by: Christopher Serr <christopher.serr@gmail.com>

compiler/rustc_parse/src/parser/item.rs

workingjubilee · 2024-09-20T01:01:56Z

genuinely not sure what is going on there with that parse error going away....

workingjubilee · 2024-09-20T01:05:01Z

probably because these recoveries are in the wrong place, as discussed.

rust-log-analyzer · 2024-09-20T01:32:57Z

The job x86_64-gnu-llvm-18 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

------
 > importing cache manifest from ghcr.io/rust-lang/rust-ci-cache:30ca74372d8b771363f68f939a58b017a592fae4f69398600dc51145997160f03e9652051f957840c41898984a88855e9757fa23464703a5a4ba21316ddebb04:
------
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-18]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-18', '--enable-llvm-link-shared', '--set', 'rust.randomize-layout=true', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-18/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.randomize-layout := True
configure: rust.thin-lto-import-instr-limit := 10
---

9    = help: pass `--edition 2021` to `rustc`
10    = note: for more on editions, read https://doc.rust-lang.org/edition-guide
11 
- error: expected item, found `"owned_box"`
-    |
-    |
- LL |     s#[c"owned_box"]
-    |
-    = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>
- 
- error: aborting due to 2 previous errors
---
To only update this specific test, also pass `--test-args parser/help-set-edition-ice-122130.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/parser/help-set-edition-ice-122130.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/parser/help-set-edition-ice-122130" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/parser/help-set-edition-ice-122130/auxiliary"
--- stderr -------------------------------
--- stderr -------------------------------
error: expected one of `(`, `,`, `=`, `{`, or `}`, found `#`
   |
   |
LL |     s#[c"owned_box"]
   |      ^ expected one of `(`, `,`, `=`, `{`, or `}`
   = note: you may be trying to write a c-string literal
   = note: you may be trying to write a c-string literal
   = note: c-string literals require Rust 2021 or later
   = help: pass `--edition 2021` to `rustc`
   = note: for more on editions, read https://doc.rust-lang.org/edition-guide
error: aborting due to 1 previous error
------------------------------------------

workingjubilee · 2024-09-23T15:27:04Z

It seems that the code for the entire "parse const item" arm inside parse_item_kind studiously avoids ever parsing a const block, meaning that one cannot try to reparse (and thus diagnose) a "failed" const item inside it. sigh.

rustbot assigned BoxyUwU Jul 29, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 29, 2024

CryZe reviewed Jul 29, 2024

View reviewed changes

compiler/rustc_parse/src/parser/item.rs Outdated Show resolved Hide resolved

CryZe reviewed Jul 29, 2024

View reviewed changes

tests/ui/inline-const/expr-in-item-context.stderr Outdated Show resolved Hide resolved

compiler-errors reviewed Jul 29, 2024

View reviewed changes

compiler-errors assigned compiler-errors and unassigned BoxyUwU Jul 29, 2024

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 30, 2024

workingjubilee and others added 3 commits September 19, 2024 16:48

tests: overly-terse inline-const-as-item error

d03b5b4

compiler: diagnose anon const items written as exprs

c55ed32

compiler: do not write const _: () backwards

7bfec85

Co-authored-by: Christopher Serr <christopher.serr@gmail.com>

workingjubilee force-pushed the fix-terse-const-item-diagnostic branch from 0cab476 to 7bfec85 Compare September 19, 2024 23:50

workingjubilee marked this pull request as draft September 19, 2024 23:50

This comment was marked as outdated.

Sign in to view

Defuse the bomb?

bf6064b

compiler-errors reviewed Sep 20, 2024

View reviewed changes

compiler/rustc_parse/src/parser/item.rs Outdated Show resolved Hide resolved

compiler/rustc_parse/src/parser/item.rs Outdated Show resolved Hide resolved

Snapshot to reduce variance?

fe00fde

This comment has been minimized.

Sign in to view

compiler: Structure the anon-const-not-inline suggestion

f422345

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compiler: suggest `const _` for a misplaced `const {}` #128374

compiler: suggest `const _` for a misplaced `const {}` #128374

workingjubilee commented Jul 29, 2024

rustbot commented Jul 29, 2024

compiler-errors Jul 29, 2024

workingjubilee Jul 29, 2024

compiler-errors Jul 30, 2024

workingjubilee Jul 30, 2024

compiler-errors Jul 30, 2024

compiler-errors Jul 29, 2024

workingjubilee Jul 30, 2024

compiler-errors Jul 30, 2024

workingjubilee Jul 30, 2024

compiler-errors Jul 29, 2024

workingjubilee Jul 30, 2024

compiler-errors Jul 30, 2024

This comment has been minimized.

workingjubilee commented Jul 30, 2024

compiler-errors commented Jul 30, 2024

workingjubilee commented Jul 30, 2024

This comment was marked as outdated.

This comment has been minimized.

workingjubilee commented Sep 20, 2024

workingjubilee commented Sep 20, 2024

rust-log-analyzer commented Sep 20, 2024

workingjubilee commented Sep 23, 2024

compiler: suggest const _ for a misplaced const {} #128374

Are you sure you want to change the base?

compiler: suggest const _ for a misplaced const {} #128374

Conversation

workingjubilee commented Jul 29, 2024

rustbot commented Jul 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

workingjubilee commented Jul 30, 2024

compiler-errors commented Jul 30, 2024

workingjubilee commented Jul 30, 2024

This comment was marked as outdated.

This comment has been minimized.

workingjubilee commented Sep 20, 2024

workingjubilee commented Sep 20, 2024

rust-log-analyzer commented Sep 20, 2024

workingjubilee commented Sep 23, 2024

compiler: suggest `const _` for a misplaced `const {}` #128374

compiler: suggest `const _` for a misplaced `const {}` #128374