Switch EvalLookupSingleImplWitness from "concrete" to "final" terminology (#6246)

A `final impl` can have a symbolic witness, but that witness is still
final. Using "final" here per discussion on
[#generics-and-templates](https://discord.com/channels/655572317891461132/941071822756143115/1428851511672312120).

I'm also changing the variant a little because `concrete_witness` was
only called when `has_concrete_value` was true, so it can be more
careful about its contract. Having a more explicit `None` also
simplifies `has_value`. I think it doesn't change the overall cost much
past that.

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()

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);

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;
     }

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<FoundNonFinalImpl>(result_) ||
-           std::get<SemIR::InstId>(result_).has_value();
+    return !std::holds_alternative<FoundNone>(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<SemIR::InstId>(&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<SemIR::InstId>(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<SemIR::InstId>(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<SemIR::InstId>(value_);
   }
 
  private:
+  struct FoundNone {};
   struct FoundNonFinalImpl {};
+  using Value = std::variant<SemIR::InstId, FoundNone, FoundNonFinalImpl>;
 
-  explicit EvalImplLookupResult(SemIR::InstId inst_id) : result_(inst_id) {}
-  explicit EvalImplLookupResult(FoundNonFinalImpl f) : result_(f) {}
+  explicit EvalImplLookupResult(Value value) : value_(value) {}
 
-  std::variant<SemIR::InstId, FoundNonFinalImpl> 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