-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Point at span within local macros even when error happens in nested external macro #148717
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
|
cc @Muscraft |
|
rustbot has assigned @petrochenkov. Use |
This comment has been minimized.
This comment has been minimized.
| | | ||
| help: the trait `ConstParamTy_` is implemented for `()` | ||
| --> $SRC_DIR/core/src/marker.rs:LL:COL | ||
| ::: $SRC_DIR/core/src/marker.rs:LL:COL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what's going on here.
The span is external, why is it reported? Because we cannot find its local parent call site?
Why is it reported twice now?
A number of tests show improvements similar to the macro-span-caller-replacement.rs case, but more tests show changes like this, which don't seem right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is caused by a bug/difference in how AnnotateSnippetEmitter handles annotations from files whose source is unavailable ("unannotated messages"). I have some changes locally that will hopefully address this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The span is external, why is it reported? Because we cannot find its local parent call site?
Why is it reported twice now?
I think it is because it is now considered local from the point of view of the current module being shown, not from the span in the main subdiagnostic. There are now two spans because there's one pointing inside the macro and one pointing at the macro call. (You can see this when running these tests in an environment that has the std files available for rendering.)
I have some changes locally that will hopefully address this issue.
Are you also looking at making the ordering consistent? I don't think we used to do that. I also noticed that when we have multiple primary spans within different macros, we only point at a single macro call. I'm assuming that MultiSpan never accounted for its component Spans having different contexts. I wouldn't worry too much about that for now, but it's good that this change surfaced that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you also looking at making the ordering consistent? I don't think we used to do that.
I'm not sure what ordering you are referring to here, but I can look into it if you give me a couple of examples.
I also noticed that when we have multiple primary spans within different macros, we only point at a single macro call. I'm assuming that
MultiSpannever accounted for its componentSpans having different contexts. I wouldn't worry too much about that for now, but it's good that this change surfaced that.
Could you also provide a couple of examples of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For context, this is what it looks like in a sane std environment:
error[E0277]: `*mut ()` can't be used as a const parameter type
--> tests/ui/const-generics/adt_const_params/const_param_ty_bad.rs:11:11
|
11 | check(&mut () as *mut ()); //~ error: `*mut ()` can't be used as a const parameter type
| ----- ^^^^^^^^^^^^^^^^^^ the trait `ConstParamTy_` is not implemented for `*mut ()`
| |
| required by a bound introduced by this call
|
help: the trait `ConstParamTy_` is implemented for `()`
--> /home/gh-estebank/workspace/rust/library/core/src/marker.rs:60:26
|
60 | $(#[$($meta)*])* impl< $($($bounds)*)? > $Trait for $T {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
1098 | / marker_impls! {
1099 | | #[unstable(feature = "adt_const_params", issue = "95174")]
1100 | | ConstParamTy_ for
1101 | | usize, u8, u16, u32, u64, u128,
... |
1106 | | {T: ConstParamTy_, const N: usize} [T; N],
1107 | | }
| |_- in this macro invocation
note: required by a bound in `check`
--> tests/ui/const-generics/adt_const_params/const_param_ty_bad.rs:4:18
|
4 | fn check(_: impl std::marker::ConstParamTy_) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `check`
= note: this error originates in the macro `marker_impls` (in Nightly builds, run with -Z macro-backtrace for more info)
And this is what it currently looks like:
error[E0277]: `*mut ()` can't be used as a const parameter type
--> tests/ui/const-generics/adt_const_params/const_param_ty_bad.rs:11:11
|
11 | check(&mut () as *mut ()); //~ error: `*mut ()` can't be used as a const parameter type
| ----- ^^^^^^^^^^^^^^^^^^ the trait `ConstParamTy_` is not implemented for `*mut ()`
| |
| required by a bound introduced by this call
|
help: the trait `ConstParamTy_` is implemented for `()`
--> /home/gh-estebank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/marker.rs:1098:1
|
1098 | / marker_impls! {
1099 | | #[unstable(feature = "adt_const_params", issue = "95174")]
1100 | | ConstParamTy_ for
1101 | | usize, u8, u16, u32, u64, u128,
... |
1106 | | {T: ConstParamTy_, const N: usize} [T; N],
1107 | | }
| |_^
note: required by a bound in `check`
--> tests/ui/const-generics/adt_const_params/const_param_ty_bad.rs:4:18
|
4 | fn check(_: impl std::marker::ConstParamTy_) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `check`
= note: this error originates in the macro `marker_impls` (in Nightly builds, run with -Z macro-backtrace for more info)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example does look convincing, it's unfortunate that our tests suite is not representative here.
But it would be similarly annoying to update tests outputs on miscellaneous changes to the standard library, so it seems similar to LL:COL line number anonymization.
This comment was marked as outdated.
This comment was marked as outdated.
51fb73b to
e3e03ee
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
9e04a9e to
99a13db
Compare
This comment has been minimized.
This comment has been minimized.
|
r=me with CI fixed and commits squashed. |
|
Reminder, once the PR becomes ready for a review, use |
99a13db to
7339e67
Compare
|
The Miri subtree was changed cc @rust-lang/miri |
| // Backtraces vary wildly between platforms, we have to normalize away almost the entire thing | ||
| //@normalize-stderr-test: "'main'|'<unnamed>'" -> "$$NAME" | ||
| //@normalize-stderr-test: ".*(note|-->|\|).*\n" -> "" | ||
| //@normalize-stderr-test: ".*(note|-->|:::|\|).*\n" -> "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this need normalization? Does the output differ between targets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it: f3b52fd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, I misread the regex. Makes sense, thanks.
|
@bors r=petrochenkov |
…nkov Point at span within local macros even when error happens in nested external macro Address issue noticed at https://users.rust-lang.org/t/error-message-does-not-specify-where-in-macro/135157/1. On errors occurring within a macro expansion, point at the innermost local macro expansion point. ``` error[E0308]: mismatched types --> $DIR/macro-span-caller-replacement.rs:5:17 | LL | s = format!("{arg}"); | ^^^^^^^^^^^^^^^^ expected `&str`, found `String` ... LL | macro_with_format!(); | -------------------- in this macro invocation | = note: this error originates in the macro `format` which comes from the expansion of the macro `macro_with_format` (in Nightly builds, run with -Z macro-backtrace for more info) ```
Rollup of 6 pull requests Successful merges: - #147602 (Deduplicate higher-ranked lifetime capture errors in impl Trait) - #147725 (Remove -Zoom=panic) - #148491 ( Correctly provide suggestions when encountering `async fn` with a `dyn Trait` return type) - #148717 (Point at span within local macros even when error happens in nested external macro) - #149458 (Run clippy on cg_gcc in CI) - #149816 (Make typo in field and name suggestions verbose) r? `@ghost` `@rustbot` modify labels: rollup
|
Possibly failed in rollup: #149820 (comment) @bors r- |
|
@bors try jobs=x86_64-gnu-llvm-21-3 |
This comment has been minimized.
This comment has been minimized.
Point at span within local macros even when error happens in nested external macro try-job: x86_64-gnu-llvm-21-3
This comment has been minimized.
This comment has been minimized.
|
💔 Test for e546dea failed: CI. Failed jobs:
|
…xternal macro
```
error[E0308]: mismatched types
--> $DIR/macro-span-caller-replacement.rs:5:17
|
LL | s = format!("{arg}");
| ^^^^^^^^^^^^^^^^ expected `&str`, found `String`
...
LL | macro_with_format!();
| -------------------- in this macro invocation
|
= note: this error originates in the macro `format` which comes from the expansion of the macro `macro_with_format` (in Nightly builds, run with -Z macro-backtrace for more info)
```
7339e67 to
1bd7934
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. |
|
@bors r=petrochenkov |
…nkov Point at span within local macros even when error happens in nested external macro Address issue noticed at https://users.rust-lang.org/t/error-message-does-not-specify-where-in-macro/135157/1. On errors occurring within a macro expansion, point at the innermost local macro expansion point. ``` error[E0308]: mismatched types --> $DIR/macro-span-caller-replacement.rs:5:17 | LL | s = format!("{arg}"); | ^^^^^^^^^^^^^^^^ expected `&str`, found `String` ... LL | macro_with_format!(); | -------------------- in this macro invocation | = note: this error originates in the macro `format` which comes from the expansion of the macro `macro_with_format` (in Nightly builds, run with -Z macro-backtrace for more info) ```
…uwer Rollup of 10 pull requests Successful merges: - #142380 (Put negative implementors first and apply same ordering logic to foreign implementors) - #146584 (remove duplicated columns from `rustc_error_code::error_codes!`) - #148717 (Point at span within local macros even when error happens in nested external macro) - #148825 (Add SystemTime::{MIN, MAX}) - #149565 (rustdoc: Add unstable `--merge-doctests=yes/no/auto` flag) - #149770 (Rename some issue-* tests) - #149807 (Use ubuntu:24.04 for the `x86_64-gnu-miri` job) - #149850 (Remove "tidy" tool for `tests/rustdoc` testsuite) - #149863 (Do not suggest moving expression out of for loop when hitting `break` from desugaring) - #149867 (only resolve main in bin crates) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - #142380 (Put negative implementors first and apply same ordering logic to foreign implementors) - #146584 (remove duplicated columns from `rustc_error_code::error_codes!`) - #148717 (Point at span within local macros even when error happens in nested external macro) - #149565 (rustdoc: Add unstable `--merge-doctests=yes/no/auto` flag) - #149770 (Rename some issue-* tests) - #149807 (Use ubuntu:24.04 for the `x86_64-gnu-miri` job) - #149850 (Remove "tidy" tool for `tests/rustdoc` testsuite) - #149863 (Do not suggest moving expression out of for loop when hitting `break` from desugaring) - #149867 (only resolve main in bin crates) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - #142380 (Put negative implementors first and apply same ordering logic to foreign implementors) - #146584 (remove duplicated columns from `rustc_error_code::error_codes!`) - #148717 (Point at span within local macros even when error happens in nested external macro) - #149565 (rustdoc: Add unstable `--merge-doctests=yes/no/auto` flag) - #149770 (Rename some issue-* tests) - #149807 (Use ubuntu:24.04 for the `x86_64-gnu-miri` job) - #149850 (Remove "tidy" tool for `tests/rustdoc` testsuite) - #149863 (Do not suggest moving expression out of for loop when hitting `break` from desugaring) - #149867 (only resolve main in bin crates) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #148717 - estebank:macro-spans-2, r=petrochenkov Point at span within local macros even when error happens in nested external macro Address issue noticed at https://users.rust-lang.org/t/error-message-does-not-specify-where-in-macro/135157/1. On errors occurring within a macro expansion, point at the innermost local macro expansion point. ``` error[E0308]: mismatched types --> $DIR/macro-span-caller-replacement.rs:5:17 | LL | s = format!("{arg}"); | ^^^^^^^^^^^^^^^^ expected `&str`, found `String` ... LL | macro_with_format!(); | -------------------- in this macro invocation | = note: this error originates in the macro `format` which comes from the expansion of the macro `macro_with_format` (in Nightly builds, run with -Z macro-backtrace for more info) ```
Rollup of 9 pull requests Successful merges: - rust-lang/rust#142380 (Put negative implementors first and apply same ordering logic to foreign implementors) - rust-lang/rust#146584 (remove duplicated columns from `rustc_error_code::error_codes!`) - rust-lang/rust#148717 (Point at span within local macros even when error happens in nested external macro) - rust-lang/rust#149565 (rustdoc: Add unstable `--merge-doctests=yes/no/auto` flag) - rust-lang/rust#149770 (Rename some issue-* tests) - rust-lang/rust#149807 (Use ubuntu:24.04 for the `x86_64-gnu-miri` job) - rust-lang/rust#149850 (Remove "tidy" tool for `tests/rustdoc` testsuite) - rust-lang/rust#149863 (Do not suggest moving expression out of for loop when hitting `break` from desugaring) - rust-lang/rust#149867 (only resolve main in bin crates) r? `@ghost` `@rustbot` modify labels: rollup
Address issue noticed at https://users.rust-lang.org/t/error-message-does-not-specify-where-in-macro/135157/1. On errors occurring within a macro expansion, point at the innermost local macro expansion point.