#56411 do not suggest a fix for a import conflict in a macro#56937
#56411 do not suggest a fix for a import conflict in a macro#56937mockersf wants to merge 1 commit intorust-lang:masterfrom mockersf:#56411-ICE-import-macro
Conversation
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
estebank
left a comment
There was a problem hiding this comment.
One nitpick, otherwise fine by me.
|
|
||
| struct T {} | ||
|
|
||
| fn main() {} |
There was a problem hiding this comment.
These files usually go in an auxiliary directory. I'm ok with this if @petrochenkov is too.
There was a problem hiding this comment.
yes, but // aux-build: would allow to build it as an extern crate, not to use it as a module. And the auxiliary folder doesn't contain a mod.rs file, so you can't import a module from it...
There was a problem hiding this comment.
Could you rename the file into something less subtle than "same file, but with underscore", e.g. issue_56411_aux.rs?
| cm.span_to_snippet(binding.span), | ||
| binding.kind.clone(), | ||
| binding.span.is_dummy(), | ||
| binding.span.ctxt().outer().expn_info().is_some(), |
There was a problem hiding this comment.
If you rely on source.span.hi().0 - binding.span.lo().0 being a valid index in binding.span, then you should check at least the next conditions - !source.span.is_dummy() && !binding.span.is_dummy() && !from_macro(source.span) && !from_macro(binding.span).
There was a problem hiding this comment.
Actually, directly checking that source.span.hi().0 - binding.span.lo().0 doesn't underflow and results in a valid index into cm would be the most reliable alternative.
This way we certainly don't get an ICE, but also keep the diagnostics for "good" macros.
|
ping from triage @mockersf waiting for your reply on the review comments |
|
Ping from triage @mockersf: Some changes have been requested on this PR. Do you think you'll be able to take care of those? |
|
Rebased and fixed in #57908 |
resolve: Fix span arithmetics in the import conflict error rust-lang#56937 rebased and fixed Fixes rust-lang#56411 Fixes rust-lang#57071 Fixes rust-lang#57787 r? @estebank
resolve: Fix span arithmetics in the import conflict error rust-lang#56937 rebased and fixed Fixes rust-lang#56411 Fixes rust-lang#57071 Fixes rust-lang#57787 r? @estebank
resolve: Fix span arithmetics in the import conflict error rust-lang#56937 rebased and fixed Fixes rust-lang#56411 Fixes rust-lang#57071 Fixes rust-lang#57787 r? @estebank
as a first fix, remove the suggestion when the conflict happens in a macro as it will be useless