pattern analysis: Use contiguous indices for enum variants #16979
Conversation
rustbot
added
the
S-waiting-on-review
| &Variant(id) => Some(id.into()), | ||
| Variant(id) => { | ||
| let hir_def::AdtId::EnumId(eid) = adt else { | ||
| panic!("bad constructor {ctor:?} for adt {adt:?}") |
There was a problem hiding this comment.
Maybe stdx::never and return None here so we don't panic?
There was a problem hiding this comment.
If we return None we panic only a tiny bit later because most of the calls of this function unwrap() the output
There was a problem hiding this comment.
Want me to change that anyway?
There was a problem hiding this comment.
panicking early seems better then
Nadrieril
force-pushed
the
contiguous-enum-id
branch
from
0a84c37
to
216dab3
Compare
| // Find the index of this variant in the list of variants. | ||
| db.enum_data(eid) | ||
| .variants | ||
| .iter() | ||
| .map(|(evid, _name)| *evid) | ||
| .enumerate() | ||
| .find(|(_, evid)| *evid == target_evid) | ||
| .map(|(i, _)| EnumVariantContiguousIndex(i)) |
There was a problem hiding this comment.
This can be simplified to
| // Find the index of this variant in the list of variants. | |
| db.enum_data(eid) | |
| .variants | |
| .iter() | |
| .map(|(evid, _name)| *evid) | |
| .enumerate() | |
| .find(|(_, evid)| *evid == target_evid) | |
| .map(|(i, _)| EnumVariantContiguousIndex(i)) | |
| target_evid.lookup(self.db.upcast()).index as usize |
(we store the index in the item tree to prevent a linear lookup here)
|
Thanks! |
|
✌️ @Nadrieril, you can now approve this pull request! If @Veykril told you to " |
Nadrieril
force-pushed
the
contiguous-enum-id
branch
from
216dab3
to
7e8f2d8
Compare
|
☀️ Test successful - checks-actions |
The main blocker to using the in-tree version of the
pattern_analysiscrate is that rustc requires enum indices to be contiguous because it usesIndexVec/BitSetfor performance. Currently we swap these out forFxHashMap/FxHashSetwhen therustcfeature is off, but we can't do that if we use the in-tree crate.This PR solves the problem by using contiguous indices on the r-a side too.