-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,6 +84,9 @@ LL | check(&mut () as *mut ()); | |
| | | ||
| help: the trait `ConstParamTy_` is implemented for `()` | ||
| --> $SRC_DIR/core/src/marker.rs:LL:COL | ||
| ::: $SRC_DIR/core/src/marker.rs:LL:COL | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? A number of tests show improvements similar to the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is caused by a bug/difference in how
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.)
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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
Could you also provide a couple of examples of this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: And this is what it currently looks like:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| | | ||
| = note: in this macro invocation | ||
| note: required by a bound in `check` | ||
| --> $DIR/const_param_ty_bad.rs:4:18 | ||
| | | ||
|
|
@@ -101,6 +104,9 @@ LL | check(&() as *const ()); | |
| | | ||
| help: the trait `ConstParamTy_` is implemented for `()` | ||
| --> $SRC_DIR/core/src/marker.rs:LL:COL | ||
| ::: $SRC_DIR/core/src/marker.rs:LL:COL | ||
| | | ||
| = note: in this macro invocation | ||
| note: required by a bound in `check` | ||
| --> $DIR/const_param_ty_bad.rs:4:18 | ||
| | | ||
|
|
||
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.