diff --git a/toolchain/check/check_unit.cpp b/toolchain/check/check_unit.cpp index 6e5d143ed76d7..e1aca867a99ba 100644 --- a/toolchain/check/check_unit.cpp +++ b/toolchain/check/check_unit.cpp @@ -526,9 +526,9 @@ auto CheckUnit::CheckPoisonedConcreteImplLookupQueries() -> void { for (const auto& poison : poisoned_queries) { auto witness_result = EvalLookupSingleImplWitness( context_, poison.loc_id, poison.query, poison.query.query_self_inst_id, - /*poison_concrete_results=*/false); - CARBON_CHECK(witness_result.has_concrete_value()); - auto found_witness_id = witness_result.concrete_witness(); + /*poison_final_results=*/false); + CARBON_CHECK(witness_result.has_final_value()); + auto found_witness_id = witness_result.final_witness(); if (found_witness_id != poison.impl_witness) { auto witness_to_impl_id = [&](SemIR::InstId witness_id) { auto table_id = context_.insts() diff --git a/toolchain/check/eval_inst.cpp b/toolchain/check/eval_inst.cpp index a7b7be1cd977b..64aacad2775f0 100644 --- a/toolchain/check/eval_inst.cpp +++ b/toolchain/check/eval_inst.cpp @@ -295,7 +295,7 @@ auto EvalConstantInst(Context& context, SemIR::InstId inst_id, auto result = EvalLookupSingleImplWitness(context, SemIR::LocId(inst_id), inst, self_facet_value_inst_id, - /*poison_concrete_results=*/true); + /*poison_final_results=*/true); if (!result.has_value()) { // We use NotConstant to communicate back to impl lookup that the lookup // failed. This can not happen for a deferred symbolic lookup in a generic @@ -303,9 +303,9 @@ auto EvalConstantInst(Context& context, SemIR::InstId inst_id, // evaluated here) to the SemIR if the lookup succeeds. return ConstantEvalResult::NotConstant; } - if (result.has_concrete_value()) { + if (result.has_final_value()) { return ConstantEvalResult::Existing( - context.constant_values().Get(result.concrete_witness())); + context.constant_values().Get(result.final_witness())); } return ConstantEvalResult::NewSamePhase(inst); diff --git a/toolchain/check/impl_lookup.cpp b/toolchain/check/impl_lookup.cpp index 33d16bc51e660..6666f92f7ad8b 100644 --- a/toolchain/check/impl_lookup.cpp +++ b/toolchain/check/impl_lookup.cpp @@ -847,14 +847,14 @@ static auto CollectCandidateImplsForQuery( auto EvalLookupSingleImplWitness(Context& context, SemIR::LocId loc_id, SemIR::LookupImplWitness eval_query, SemIR::InstId self_facet_value_inst_id, - bool poison_concrete_results) + bool poison_final_results) -> EvalImplLookupResult { auto query_specific_interface = context.specific_interfaces().Get(eval_query.query_specific_interface_id); auto facet_lookup_result = LookupImplWitnessInSelfFacetValue( context, self_facet_value_inst_id, query_specific_interface); - if (facet_lookup_result.has_concrete_value()) { + if (facet_lookup_result.has_final_value()) { return facet_lookup_result; } @@ -927,19 +927,19 @@ auto EvalLookupSingleImplWitness(Context& context, SemIR::LocId loc_id, context, loc_id, query_is_concrete, query_self_const_id, query_specific_interface, candidate.impl_id); if (result.has_value()) { - // Record the query which found a concrete impl witness. It's illegal to + // Record the query which found a final impl witness. It's illegal to // write a final impl afterward that would match the same query. // // If the impl was effectively final, then we don't need to poison here. A // change of query result will already be diagnosed at the point where the // new impl decl was written that changes the result. - if (poison_concrete_results && result.has_concrete_value() && + if (poison_final_results && result.has_final_value() && !IsImplEffectivelyFinal(context, context.impls().Get(candidate.impl_id))) { context.poisoned_concrete_impl_lookup_queries().push_back( {.loc_id = loc_id, .query = eval_query, - .impl_witness = result.concrete_witness()}); + .impl_witness = result.final_witness()}); } return result; } diff --git a/toolchain/check/impl_lookup.h b/toolchain/check/impl_lookup.h index e9572971befb4..d86e10b3583f3 100644 --- a/toolchain/check/impl_lookup.h +++ b/toolchain/check/impl_lookup.h @@ -49,15 +49,16 @@ auto LookupMatchesImpl(Context& context, SemIR::LocId loc_id, // The result of EvalLookupSingleImplWitness(). It can be one of: // - No value. Lookup failed to find an impl declaration. -// - A concrete value. Lookup found a concrete impl declaration that can be -// used definitively. -// - A symbolic value. Lookup found an impl but it is not returned since lookup -// will need to be done again with a more specific query to look for -// specializations. +// - An effectively final value. Lookup found either a concrete impl or a +// `final` impl declaration; both can be used definitely. A witness is +// available. +// - A non-`final` symbolic value. Lookup found an impl, but it is not returned +// since lookup will need to be done again with a more specific query to look +// for specializations. class [[nodiscard]] EvalImplLookupResult { public: static auto MakeNone() -> EvalImplLookupResult { - return EvalImplLookupResult(SemIR::InstId::None); + return EvalImplLookupResult(FoundNone()); } static auto MakeFinal(SemIR::InstId inst_id) -> EvalImplLookupResult { return EvalImplLookupResult(inst_id); @@ -66,35 +67,33 @@ class [[nodiscard]] EvalImplLookupResult { return EvalImplLookupResult(FoundNonFinalImpl()); } - // True if a concrete impl witness was found or a non-final impl. In the - // latter case the InstId of the impl's witness is not returned, only the fact - // that it exists. + // True if an impl declaration was found, either effectively final or + // symbolic. auto has_value() const -> bool { - return std::holds_alternative(result_) || - std::get(result_).has_value(); + return !std::holds_alternative(value_); } - // True if there is a concrete witness in the result. If false, and - // `has_value()` is true, it means a non-final impl was found and a further - // more specific query will need to be done. - auto has_concrete_value() const -> bool { - const auto* inst_id = std::get_if(&result_); - return inst_id && inst_id->has_value(); + // True if there is an effectively final witness in the result. If false, and + // `has_value()` is true, it means an impl was found that's not effectively + // final, and a further more specific query will need to be done. + auto has_final_value() const -> bool { + return std::holds_alternative(value_); } - // Only valid if `has_concrete_value()` is true. Returns the witness id for - // the found impl declaration, or None if `has_value()` is false. - auto concrete_witness() const -> SemIR::InstId { - return std::get(result_); + // Returns the witness id for an effectively final value's impl declaration. + // Only valid to call if `has_final_value` is true. + auto final_witness() const -> SemIR::InstId { + return std::get(value_); } private: + struct FoundNone {}; struct FoundNonFinalImpl {}; + using Value = std::variant; - explicit EvalImplLookupResult(SemIR::InstId inst_id) : result_(inst_id) {} - explicit EvalImplLookupResult(FoundNonFinalImpl f) : result_(f) {} + explicit EvalImplLookupResult(Value value) : value_(value) {} - std::variant result_; + Value value_; }; // Looks for a witness instruction of an impl declaration for a query consisting @@ -102,10 +101,13 @@ class [[nodiscard]] EvalImplLookupResult { // execute lookup via the LookupImplWitness instruction. It does not consider // the self facet value for finding a witness, since LookupImplWitness() would // have found that and not caused us to defer lookup to here. +// +// `poison_final_results` poisons lookup results which are effectively final, +// preventing overlapping final impls. auto EvalLookupSingleImplWitness(Context& context, SemIR::LocId loc_id, SemIR::LookupImplWitness eval_query, SemIR::InstId self_facet_value_inst_id, - bool poison_concrete_results) + bool poison_final_results) -> EvalImplLookupResult; } // namespace Carbon::Check