Fix unnamed lifetime spans#125234
Conversation
- More than one unnamed lifetimes gave incorrect spans when lookup was done by name (since they both had name "'_"). Use DefId instead to get correct spans.
|
rustbot has assigned @compiler-errors. Use |
rustbot
added
S-waiting-on-review
compiler/rustc_hir/src/hir.rs
Outdated
| self.params.iter().find(|¶m| name == param.name.ident().name) | ||
| } | ||
|
|
||
| pub fn get_with_def_id(&self, id: DefId) -> Option<&GenericParam<'hir>> { |
There was a problem hiding this comment.
This is definitely not correct if DefId comes from a different crate. Please make this take a LocalDefId, or ideally, do the comparison with HirId instead.
There was a problem hiding this comment.
I was not able to find a way to convert a DefId to a HirId, so I used a LocalDefId instead. The place where I use it already has expect_local() on the binding scope, so presumably the lifetime should also be local. However, I tried to use the same logic for late bound regions, and that caused a panic in a test for trying to unpack a non-local DefId. Is there a way to get the HirId?
rustbot
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #125468) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bovinebuddha any updates on this? thanks |
|
@bovinebuddha |
6 participants
Fixes issue #125143.
The error message used to mix up two different unnamed lifetimes since they were both assigned the name "'_". Looking up the span using the DefId instead gave correct spans.
I am not sure if a similar fix should be made for any of the other RegionKinds, e.g. ReLateParam, and I am unsure of how to construct a test case which used late bound lifetimes or RePlaceholder.
Maybe the function 'msg_span_from_named_region' should be renamed since it now uses (partially) DefId instead of the name. Also, the name is quite misleading since it is sometimes used to look up unnamed lifetimes...