mirror of
https://github.com/carbon-language/carbon-lang.git
synced 2026-10-05 13:11:05 +01:00
Always use LookupImplWitness instructions for symbolic witnesses (#5321)
We eliminate the `FacetAccessWitness` instruction, which would sometimes immediately evaluate to a concrete `ImplWitness`, and sometimes remain symbolic. This instruction is now replaced by `LookupImplWitness` in all cases. To support the same use cases, when it is evaluated, `LookupImplWitness` will look in the self value if it's a facet value, and attempt to return a concrete `ImplWitness` from it before looking for an `impl` statement. The `LookupImplWitness` instruction's value is now canonical, even when it evaluates to a symbolic `LookupImplWitness` instruction, by canonicalizing the self value of the lookup query. This canonicalization unwraps `FacetAccessType` and `FacetValue` instructions to get to an underlying canonical facet value. However we must preserve and use the non-canonical query while evaluating the instruction in order to look for a concrete `ImplWitness` if the query self value was a concrete `FacetValue`. The canonicalization ensures that symbolic witnesses obtained from a facet value are compatible with those obtained from an impl statement, as long as the self types originate from the same canonical facet value though they may have been narrowed. Member access now unconditionally does a `LookupImplWitness()` operation, instead of only sometimes doing the lookup for a final impl declaration. `EvalImplLookupResult` is marked `[[nodiscard]]` so that we don't construct it and forget to return it. This was a mistake made at one point during the creation of this PR. And the `has_concrete_value()` method no longer has a precondition that `has_value()` is true, since we want to look for a concrete result only in the new use of `EvalImplLookupResult` returned from lookup into the query self facet value. The TODO from `FacetAccessWitness` evaluation is addressed by ensuring the index of the witness in the `FacetValue` comes from the required interfaces of the `FacetValue`'s type, and that the type (a `FacetType`) is the same facet type used in the query to construct the `FacetValue`'s witness block. This is made possible by eliminating the `FacetAccessWitness` indirection. The lookup into a `FacetValue` happens while evaluating `LookupImplWitness` and it does so directly on the self value. This gives a consistent view of the witness set and the facet type, as they both come from the same instruction. All of this with 400 less lines of code. :) --------- Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com> Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This commit is contained in:
co-authored by
josh11b
Richard Smith
parent
94dca7967b
commit
51498547c9
@@ -295,13 +295,6 @@ class Stringifier {
|
||||
step_stack_->PushInstId(inst.facet_value_inst_id);
|
||||
}
|
||||
|
||||
auto StringifyInst(SemIR::InstId /*inst_id*/, FacetAccessWitness inst)
|
||||
-> void {
|
||||
*out_ << "<witness for ";
|
||||
step_stack_->Push(inst.facet_value_inst_id, ", interface ", inst.index,
|
||||
">");
|
||||
}
|
||||
|
||||
auto StringifyInst(SemIR::InstId /*inst_id*/, FacetType inst) -> void {
|
||||
const FacetTypeInfo& facet_type_info =
|
||||
sem_ir_->facet_types().Get(inst.facet_type_id);
|
||||
@@ -412,20 +405,7 @@ class Stringifier {
|
||||
lookup->query_specific_interface_id);
|
||||
}
|
||||
|
||||
if (auto witness =
|
||||
sem_ir_->insts().TryGetAs<FacetAccessWitness>(impl_witness_id)) {
|
||||
auto witness_type_id =
|
||||
sem_ir_->insts().Get(witness->facet_value_inst_id).type_id();
|
||||
auto facet_type = sem_ir_->types().GetAs<FacetType>(witness_type_id);
|
||||
// TODO: Support != 1 interface better.
|
||||
const auto& facet_type_info =
|
||||
sem_ir_->facet_types().Get(facet_type.facet_type_id);
|
||||
if (facet_type_info.extend_constraints.size() == 1) {
|
||||
return facet_type_info.extend_constraints.front();
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: Handle other cases.
|
||||
// TODO: Handle ImplWitness.
|
||||
return std::nullopt;
|
||||
}
|
||||
|
||||
@@ -466,17 +446,17 @@ class Stringifier {
|
||||
")");
|
||||
}
|
||||
|
||||
if (auto witness =
|
||||
sem_ir_->insts().TryGetAs<FacetAccessWitness>(witness_inst_id)) {
|
||||
if (auto lookup =
|
||||
sem_ir_->insts().TryGetAs<LookupImplWitness>(witness_inst_id)) {
|
||||
bool period_self = false;
|
||||
if (auto sym_name = sem_ir_->insts().TryGetAs<BindSymbolicName>(
|
||||
witness->facet_value_inst_id)) {
|
||||
lookup->query_self_inst_id)) {
|
||||
auto name_id =
|
||||
sem_ir_->entity_names().Get(sym_name->entity_name_id).name_id;
|
||||
period_self = (name_id == SemIR::NameId::PeriodSelf);
|
||||
}
|
||||
if (!period_self) {
|
||||
step_stack_->PushInstId(witness->facet_value_inst_id);
|
||||
step_stack_->PushInstId(lookup->query_self_inst_id);
|
||||
}
|
||||
} else {
|
||||
// TODO: Omit parens if not needed for precedence.
|
||||
|
||||
Reference in New Issue
Block a user