rust
rust copied to clipboard
Published
20 hours ago •
rust-lang
rust-lang
Improve diagnostic output the `non_local_definitions` lint
trafficstars
This PR improves (or at least tries to improve) the diagnostic output the non_local_definitions lint, by simplifying the wording, by adding a "sort of" explanation of bounds interaction that leak the impl...
This PR is best reviewed commit by commit and is voluntarily made a bit vague as to have a starting point to improve on.
Related to https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/non_local_defs.20wording.20improvements
cc @workingjubilee r? @estebank
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)
#17 exporting to docker image format
#17 sending tarball 29.5s done
#17 DONE 34.6s
##[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
---
---- [ui] tests/ui/proc-macro/nested-macro-rules.rs stdout ----
diff of stderr:
- warning: non-local `macro_rules!` definition, they should be avoided as they go against expectation
+ warning: non-local `macro_rules!` definition, `#[macro_export]` macro should be written at top level module
2 --> $DIR/auxiliary/nested-macro-rules.rs:7:9
4 LL | macro_rules! outer_macro {
The actual stderr differed from the expected stderr.
The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/proc-macro/nested-macro-rules/nested-macro-rules.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args proc-macro/nested-macro-rules.rs`
error: 1 errors occurred comparing output.
status: exit status: 0
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/proc-macro/nested-macro-rules.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)" "-O" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/proc-macro/nested-macro-rules/a" "-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/proc-macro/nested-macro-rules/auxiliary" "-Z" "span-debug" "-Z" "macro-backtrace" "--edition=2018"
--- stdout -------------------------------
PRINT-BANG INPUT (DISPLAY): FirstStruct
PRINT-BANG INPUT (DEBUG): TokenStream [
Ident {
ident: "FirstStruct",
span: /checkout/tests/ui/proc-macro/auxiliary/nested-macro-rules.rs:16:14: 16:25 (#6),
]
]
PRINT-ATTR INPUT (DISPLAY): struct FirstAttrStruct {}
PRINT-ATTR INPUT (DEBUG): TokenStream [
ident: "struct",
ident: "struct",
span: /checkout/tests/ui/proc-macro/auxiliary/nested-macro-rules.rs:10:32: 10:38 (#5),
Ident {
Ident {
ident: "FirstAttrStruct",
span: /checkout/tests/ui/proc-macro/auxiliary/nested-macro-rules.rs:16:27: 16:42 (#6),
Group {
delimiter: Brace,
stream: TokenStream [],
stream: TokenStream [],
span: /checkout/tests/ui/proc-macro/auxiliary/nested-macro-rules.rs:10:57: 10:59 (#5),
]
PRINT-BANG INPUT (DISPLAY): SecondStruct
PRINT-BANG INPUT (DISPLAY): SecondStruct
PRINT-BANG INPUT (DEBUG): TokenStream [
ident: "SecondStruct",
ident: "SecondStruct",
span: /checkout/tests/ui/proc-macro/nested-macro-rules.rs:23:38: 23:50 (#15),
]
]
PRINT-ATTR INPUT (DISPLAY): struct SecondAttrStruct {}
PRINT-ATTR INPUT (DEBUG): TokenStream [
ident: "struct",
ident: "struct",
span: /checkout/tests/ui/proc-macro/auxiliary/nested-macro-rules.rs:10:32: 10:38 (#14),
Ident {
ident: "SecondAttrStruct",
ident: "SecondAttrStruct",
span: /checkout/tests/ui/proc-macro/nested-macro-rules.rs:23:52: 23:68 (#15),
Group {
delimiter: Brace,
stream: TokenStream [],
stream: TokenStream [],
span: /checkout/tests/ui/proc-macro/auxiliary/nested-macro-rules.rs:10:57: 10:59 (#14),
]
------------------------------------------
--- stderr -------------------------------
--- stderr -------------------------------
warning: non-local `macro_rules!` definition, `#[macro_export]` macro should be written at top level module
##[warning] --> /checkout/tests/ui/proc-macro/auxiliary/nested-macro-rules.rs:7:9
LL | macro_rules! outer_macro {
| ------------------------ in this expansion of `nested_macro_rules::outer_macro!`
...
LL | / macro_rules! inner_macro {
LL | / macro_rules! inner_macro {
LL | | ($bang_macro:ident, $attr_macro:ident) => {
LL | | $bang_macro!($name);
LL | | #[$attr_macro] struct $attr_struct_name {}
LL | | }
| |_________^
|
::: /checkout/tests/ui/proc-macro/nested-macro-rules.rs:23:5
::: /checkout/tests/ui/proc-macro/nested-macro-rules.rs:23:5
|
LL | nested_macro_rules::outer_macro!(SecondStruct, SecondAttrStruct);
|
|
= help: remove the `#[macro_export]` or move this `macro_rules!` outside the of the current function `main`
= note: a `macro_rules!` definition is non-local if it is nested inside an item and has a `#[macro_export]` attribute
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
note: the lint level is defined here
--> /checkout/tests/ui/proc-macro/nested-macro-rules.rs:8:9
|
I pushed several more commits that tries to improve the clarity the message were are trying to pass, mainly by moving the "move help message" to a separate span with some labels.
warning: non-local `impl` definition, `impl` blocks should be written at the same level as their item
--> $DIR/consts.rs:43:5
|
LL | impl Test {
| ^^^^^^^^^
|
= note: methods and assoc const are still usable outside the current expression, only `impl Local` and `impl dyn Local` are local and only if the `Local` type is at the same nesting as the `impl` block
help: move this `impl` block and outside the of the current function `main`
--> $DIR/consts.rs:43:5
|
LL | impl Test {
| ^ ---- may need to be moved as well
| _____|
| |
LL | |
LL | | fn foo() {}
LL | | }
| |_____^
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
If this was unintentional then you should revert the changes before this PR is merged. Otherwise, you can ignore this comment.
I continued to improved the diagnostic output by adding labels that points to Self type and Trait to better put emphasis on the fact that they are non-local for the lint.
With the example in https://github.com/rust-lang/rust/issues/125068#issue-2292387556, it now gives:
warning: non-local `impl` definition, `impl` blocks should be written at the same level as their item
--> a.rs:6:5
|
6 | impl Trait for Option<Local> {}
| ^^^^^-----^^^^^------^^^^^^^
| | |
| | `Option` is not local
| `Trait` is not local
|
= note: `impl` may be usable in bounds, etc. from outside the expression, which might e.g. make something constructible that previously wasn't, because it's still on a publicly-visible type
= note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`
help: move this `impl` block outside of the current function `main`
--> a.rs:6:5
|
6 | impl Trait for Option<Local> {}
| ^^^^^^^^^^^^^^^^^^^^^^-----^^^^
| |
| may need to be moved as well
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
= note: `#[warn(non_local_definitions)]` on by default
with colors
All the review comments have been addressed or have been replied to.
@rustbot ready
:umbrella: The latest upstream changes (presumably #124417) made this pull request unmergeable. Please resolve the merge conflicts.
