Resolve rewrites in facet types, looking for duplicates (#5620)

Add a facet type rewrite constraint resolution step that is run every
time a facet type is constructed, in line with the design here:
https://docs.carbon-lang.dev/docs/design/generics/appendix-rewrite-constraints.html#rewrite-constraint-resolution

The resolution has multiple steps, and this PR implements the first of
them, finding and diagnosing any duplicate rewrites to the same
associated constant.

We already diagnosed this for impl construction, now we do so for all
facet types, which includes the one used for impl construction, so this
diagnostic is a superset of the previous.

Author: danakj

toolchain/check/eval.cpp

@@ -619,7 +619,8 @@ static auto GetConstantValue(EvalContext& eval_context,
     -> SemIR::FacetTypeId {
   SemIR::FacetTypeInfo info =
       GetConstantFacetTypeInfo(eval_context, facet_type_id, phase);
-  info.Canonicalize();
+  ResolveRewriteConstraintsAndCanonicalize(eval_context.context(),
+                                           SemIR::LocId::None, info);
   // TODO: Return `facet_type_id` if we can detect nothing has changed.
   return eval_context.facet_types().Add(info);
 }
@@ -1538,7 +1539,7 @@ static auto MakeConstantForBuiltinCall(EvalContext& eval_context,
       auto info = SemIR::FacetTypeInfo::Combine(
           context.facet_types().Get(lhs_facet_type_id),
           context.facet_types().Get(rhs_facet_type_id));
-      info.Canonicalize();
+      ResolveRewriteConstraintsAndCanonicalize(context, loc_id, info);
       return MakeFacetTypeResult(eval_context.context(), info, phase);
     }
 
@@ -1971,8 +1972,8 @@ static auto IsPeriodSelf(EvalContext& eval_context, SemIR::ConstantId const_id)
 // providing a `GetConstantValue` overload for a requirement block.
 template <>
 auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
-                                        SemIR::InstId /*inst_id*/,
-                                        SemIR::Inst inst) -> SemIR::ConstantId {
+                                        SemIR::InstId inst_id, SemIR::Inst inst)
+    -> SemIR::ConstantId {
   auto typed_inst = inst.As<SemIR::WhereExpr>();
 
   Phase phase = Phase::Concrete;
@@ -2044,7 +2045,8 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
       }
     }
   }
-  info.Canonicalize();
+  ResolveRewriteConstraintsAndCanonicalize(eval_context.context(),
+                                           SemIR::LocId(inst_id), info);
   return MakeFacetTypeResult(eval_context.context(), info, phase);
 }
 

toolchain/check/facet_type.cpp

@@ -5,11 +5,13 @@
 #include "toolchain/check/facet_type.h"
 
 #include "toolchain/check/convert.h"
+#include "toolchain/check/generic.h"
 #include "toolchain/check/import_ref.h"
 #include "toolchain/check/inst.h"
 #include "toolchain/check/interface.h"
 #include "toolchain/check/type.h"
 #include "toolchain/check/type_completion.h"
+#include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/typed_insts.h"
 
 namespace Carbon::Check {
@@ -164,24 +166,10 @@ auto InitialFacetTypeImplWitness(
       continue;
     }
 
-    if (table_entry != SemIR::ImplWitnessTablePlaceholder::TypeInstId) {
-      if (table_entry != rewrite_inst_id) {
-        // TODO: Figure out how to print the two different values
-        // `const_id` & `rewrite_inst_id` in the diagnostic
-        // message.
-        CARBON_DIAGNOSTIC(
-            AssociatedConstantWithDifferentValues, Error,
-            "associated constant {0} given two different values {1} and {2}",
-            SemIR::NameId, InstIdAsConstant, InstIdAsConstant);
-        auto& assoc_const = context.associated_constants().Get(
-            assoc_constant_decl->assoc_const_id);
-        context.emitter().Emit(
-            facet_type_inst_id, AssociatedConstantWithDifferentValues,
-            assoc_const.name_id, table_entry, rewrite_inst_id);
-      }
-      table_entry = SemIR::ErrorInst::InstId;
-      continue;
-    }
+    // FacetTypes resolution disallows two rewrites to the same associated
+    // constant, so we won't ever have a facet write twice to the same position
+    // in the witness table.
+    CARBON_CHECK(table_entry == SemIR::ImplWitnessTablePlaceholder::TypeInstId);
 
     // If the associated constant has a symbolic type, convert the rewrite
     // value to that type now we know the value of `Self`.
@@ -263,4 +251,103 @@ auto AllocateFacetTypeImplWitness(Context& context,
   context.inst_blocks().ReplacePlaceholder(witness_id, empty_table);
 }
 
+auto IsPeriodSelf(Context& context, SemIR::ConstantId const_id) -> bool {
+  // This also rejects the singleton Error value as it's concrete.
+  if (!const_id.is_symbolic()) {
+    return false;
+  }
+  const auto& symbolic =
+      context.constant_values().GetSymbolicConstant(const_id);
+  // Fast early reject before doing more expensive operations.
+  if (symbolic.dependence != SemIR::ConstantDependence::PeriodSelf) {
+    return false;
+  }
+  return IsPeriodSelf(context, symbolic.inst_id);
+}
+
+auto IsPeriodSelf(Context& context, SemIR::InstId inst_id) -> bool {
+  // Unwrap the `FacetAccessType` instruction, which we get when the `.Self` is
+  // converted to `type`.
+  if (auto facet_access_type =
+          context.insts().TryGetAs<SemIR::FacetAccessType>(inst_id)) {
+    inst_id = facet_access_type->facet_value_inst_id;
+  }
+  if (auto bind_symbolic_name =
+          context.insts().TryGetAs<SemIR::BindSymbolicName>(inst_id)) {
+    const auto& bind_name =
+        context.entity_names().Get(bind_symbolic_name->entity_name_id);
+    return bind_name.name_id == SemIR::NameId::PeriodSelf;
+  }
+  return false;
+}
+
+auto ResolveRewriteConstraintsAndCanonicalize(Context& context,
+                                              SemIR::LocId loc_id,
+                                              SemIR::FacetTypeInfo& facet_type)
+    -> void {
+  // This operation sorts and dedupes the rewrite constraints. They are sorted
+  // primarily by the `lhs_id`, then by the `rhs_id`.
+  facet_type.Canonicalize();
+
+  if (facet_type.rewrite_constraints.empty()) {
+    return;
+  }
+
+  for (size_t i = 0; i < facet_type.rewrite_constraints.size() - 1; ++i) {
+    auto& constraint = facet_type.rewrite_constraints[i];
+    if (constraint.lhs_id == SemIR::ErrorInst::InstId ||
+        constraint.rhs_id == SemIR::ErrorInst::InstId) {
+      continue;
+    }
+
+    auto lhs_access =
+        context.insts().TryGetAs<SemIR::ImplWitnessAccess>(constraint.lhs_id);
+    if (!lhs_access) {
+      continue;
+    }
+    auto lhs_lookup = context.insts().TryGetAs<SemIR::LookupImplWitness>(
+        lhs_access->witness_id);
+    if (!lhs_lookup) {
+      continue;
+    }
+    if (!IsPeriodSelf(context, lhs_lookup->query_self_inst_id)) {
+      continue;
+    }
+
+    // This loop moves `i` to the last position with the same LHS value, so that
+    // we don't diagnose more than once within the same contiguous range of
+    // assignments to a single LHS value.
+    for (; i < facet_type.rewrite_constraints.size() - 1; ++i) {
+      auto& next = facet_type.rewrite_constraints[i + 1];
+      if (constraint.lhs_id != next.lhs_id) {
+        break;
+      }
+      // `constraint.lhs_id == next.lhs_id` so only check for `ErrorInst` in the
+      // RHS. On the first error, `constraint.rhs_id` is set to `ErrorInst`
+      // which prevents further diagnostics for the same LHS value due to this
+      // condition.
+      if (constraint.rhs_id != SemIR::ErrorInst::InstId &&
+          next.rhs_id != SemIR::ErrorInst::InstId) {
+        CARBON_DIAGNOSTIC(
+            AssociatedConstantWithDifferentValues, Error,
+            "associated constant {0} given two different values {1} and {2}",
+            InstIdAsConstant, InstIdAsConstant, InstIdAsConstant);
+        // TODO: It would be nice to note the places where the values are
+        // assigned but rewrite constraint instructions are from canonical
+        // constant values, and have no locations. We'd need to store a location
+        // along with them in the rewrite constraints.
+        context.emitter().Emit(loc_id, AssociatedConstantWithDifferentValues,
+                               constraint.lhs_id, constraint.rhs_id,
+                               next.rhs_id);
+      }
+      constraint.rhs_id = SemIR::ErrorInst::InstId;
+      next.rhs_id = SemIR::ErrorInst::InstId;
+    }
+  }
+
+  // Canonicalize again, as we may have inserted errors into the rewrite
+  // constraints, and these could change sorting order.
+  facet_type.Canonicalize();
+}
+
 }  // namespace Carbon::Check

toolchain/check/facet_type.h

@@ -58,6 +58,21 @@ auto AllocateFacetTypeImplWitness(Context& context,
                                   SemIR::InterfaceId interface_id,
                                   SemIR::InstBlockId witness_id) -> void;
 
+// Returns whether the constant or instruction is a `BindSymbolicName` referring
+// to `.Self`.
+//
+// Working with a `ConstantId` can be more efficient for this operation than
+// unconditionally unwrapping to the `InstId`.
+auto IsPeriodSelf(Context& context, SemIR::ConstantId const_id) -> bool;
+auto IsPeriodSelf(Context& context, SemIR::InstId inst_id) -> bool;
+
+// Perform rewrite constraint resolution for a facet type and canonicalize the
+// FacetTypeInfo. The FacetTypeInfo must not be modified after this operation.
+auto ResolveRewriteConstraintsAndCanonicalize(Context& context,
+                                              SemIR::LocId loc_id,
+                                              SemIR::FacetTypeInfo& facet_type)
+    -> void;
+
 }  // namespace Carbon::Check
 
 #endif  // CARBON_TOOLCHAIN_CHECK_FACET_TYPE_H_