Remove forward references from binding patterns #4494
Conversation
This is primarily to free up space in the BindingPattern insts, but as a side effect it moves the link between BindingPattern and its BindName out of the SemIR, and into a transient data structure in Context.
|
This PR seems to fix a pre-existing bug where we were materializing redundant copies of |
There was a problem hiding this comment.
For the weird bug fix: it looks like we were previously including the bind_name_id in the canonical identity of a SymbolicBindingPattern, so we got different constant values for different bind_name_ids -- and you can't see the difference because the formatter was not printing the MatchingInstId field.
toolchain/check/pattern_match.cpp
Outdated
| binding_pattern.bind_name_id); | ||
| auto bind_name_id = context.bind_name_cache() | ||
| .Lookup(binding_pattern.entity_name_id) | ||
| .value(); |
There was a problem hiding this comment.
Does it make sense to remove the entry from the cache here?
There was a problem hiding this comment.
I think removing it outright could mask bugs, by allowing it to be re-inserted (which should never happen), but we can at least invalidate the entry. Done.
Co-authored-by: Richard Smith <[email protected]>
This is primarily to free up space in the BindingPattern insts, but as a side effect it moves the link between BindingPattern and its BindName out of the SemIR, and into a transient data structure in Context.