rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
93.58k stars 12.05k forks source link

Fix parse error message for meta items #124778

Closed fmease closed 1 week ago

fmease commented 1 week ago

Addresses https://github.com/rust-lang/rust/issues/122796#issuecomment-2010803906, cc [@]Thomasdezeeuw.

For attrs inside of a macro like #[doc(alias = $ident)] or #[cfg(feature = $ident)] where $ident is a macro metavariable of fragment kind ident, we used to say the following when expanded (with $identident):

error: expected unsuffixed literal or identifier, found `ident`
  --> weird.rs:6:19
   |
6  |      #[cfg(feature = $ident)]
   |                      ^^^^^^
...
11 | m!(id);
   | ------ in this macro invocation
   |
   = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)

This was incorrect and caused confusion, justifiably so (see #122796).

In this position, we only accept/expect unsuffixed literals which consist of numeric & string literals as well as the boolean literals / the keywords / the reserved identifiers false & true but not arbitrary identifiers.

Furthermore, we used to suggest garbage when encountering unexpected non-identifier tokens:

error: expected unsuffixed literal, found `-`
  --> weird.rs:16:17
   |
16 | #[cfg(feature = -1)]
   |                 ^
   |
help: surround the identifier with quotation marks to parse it as a string
   |
16 | #[cfg(feature =" "-1)]
   |                + +

Now we no longer do.

rustbot commented 1 week ago

r? @lcnr

rustbot has assigned @lcnr. 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

rust-log-analyzer commented 1 week ago

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

Click to see the possible cause of the failure (guessed by this bot) ```plain #17 exporting to docker image format #17 sending tarball 30.1s done #17 DONE 32.7s ##[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-17] --- 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-17', '--enable-llvm-link-shared', '--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', '--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-17/bin/llvm-config configure: llvm.link-shared := True configure: rust.thin-lto-import-instr-limit := 10 configure: change-id := 99999999 --- 4 LL | #[deprecated(note = test)] 5 | ^^^^ 6 | - help: surround the identifier with quotation marks to parse it as a string + help: surround the identifier with quotation marks to make it parse as a string 8 | 9 LL | #[deprecated(note = "test")] The actual stderr differed from the expected stderr. Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/deprecation/issue-66340-deprecated-attr-non-meta-grammar/issue-66340-deprecated-attr-non-meta-grammar.stderr Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/deprecation/issue-66340-deprecated-attr-non-meta-grammar/issue-66340-deprecated-attr-non-meta-grammar.stderr To update references, rerun the tests and pass the `--bless` flag To only update this specific test, also pass `--test-args deprecation/issue-66340-deprecated-attr-non-meta-grammar.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/deprecation/issue-66340-deprecated-attr-non-meta-grammar.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/deprecation/issue-66340-deprecated-attr-non-meta-grammar" "-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/deprecation/issue-66340-deprecated-attr-non-meta-grammar/auxiliary" --- stderr ------------------------------- error: expected unsuffixed literal, found `test` ##[error] --> /checkout/tests/ui/deprecation/issue-66340-deprecated-attr-non-meta-grammar.rs:9:21 | | LL | #[deprecated(note = test)] | ^^^^ | help: surround the identifier with quotation marks to make it parse as a string | LL | #[deprecated(note = "test")] error: aborting due to 1 previous error ------------------------------------------ ---- [ui] tests/ui/parser/attribute/attr-unquoted-ident.rs stdout ---- thread '[ui] tests/ui/parser/attribute/attr-unquoted-ident.rs' panicked at src/tools/compiletest/src/runtest.rs:4232:13: the `//@ run-rustfix` directive wasn't found but a `*.fixed` file was found failures: [ui] tests/ui/deprecation/issue-66340-deprecated-attr-non-meta-grammar.rs [ui] tests/ui/parser/attribute/attr-unquoted-ident.rs ```
Thomasdezeeuw commented 1 week ago

LGTM. Tiny question @fmease can you remove my name from the commit message? Otherwise I'll get a GitHub ping every time it's committed to the various forks.

fmease commented 1 week ago

I only tagged you in the PR description not the commit message, so that should've been fine esp. since @bors quotes username mentions in merge commits.

Anyway, for good measure I've unlinked you from the PR description.

lcnr commented 1 week ago

r? compiler

lcnr commented 1 week ago

r? compiler

nnethercote commented 1 week ago

r=me with the suggestion considered. Thanks!

fmease commented 1 week ago

@bors r=nnethercote rollup

bors commented 1 week ago

:pushpin: Commit 0ad3c5da72469c848e321ddee207f9a5dfbf9876 has been approved by nnethercote

It is now in the queue for this repository.