Avoid crashing in custom witness for FacetTypes and symbolic object representations (#7033)

The type must be complete to look for a witness for Destroy. Do this
check through type completion rather than just checking to see if the
ClassInfo says the definition is closed, since completing the type has
side effects (resolves the self specific definition).

Then look for whether the class is abstract through the CompleteTypeInfo
instead of just looking at the inheritance type on ClassInfo, like type
completion does.

Last, FacetTypes are trivially destroyed just like TypeType.

toolchain/check/custom_witness.cpp

@@ -18,6 +18,7 @@
 #include "toolchain/sem_ir/associated_constant.h"
 #include "toolchain/sem_ir/builtin_function_kind.h"
 #include "toolchain/sem_ir/ids.h"
+#include "toolchain/sem_ir/type_info.h"
 #include "toolchain/sem_ir/typed_insts.h"
 
 namespace Carbon::Check {
@@ -133,16 +134,16 @@ static auto HasWitnessForOneField(
 // Returns true if `class_type` should impl `Destroy`.
 static auto CanDestroyClass(
     Context& context, SemIR::LocId loc_id, SemIR::ClassType class_type,
+    const SemIR::CompleteTypeInfo& complete_info,
     SemIR::SpecificInterfaceId query_specific_interface_id, bool is_partial)
     -> DestroyFormat {
-  auto class_info = context.classes().Get(class_type.class_id);
-  // Incomplete and abstract classes can't be destroyed.
-  if (!class_info.is_complete() ||
-      (!is_partial && class_info.inheritance_kind ==
-                          SemIR::Class::InheritanceKind::Abstract)) {
+  // Abstract classes can't be destroyed.
+  if (!is_partial && complete_info.IsAbstract()) {
     return DestroyFormat::NoDestroy;
   }
 
+  auto class_info = context.classes().Get(class_type.class_id);
+
   // `LookupCppImpl` handles C++ types.
   if (context.name_scopes().Get(class_info.scope_id).is_cpp_scope()) {
     return DestroyFormat::NoDestroy;
@@ -166,23 +167,33 @@ static auto CanDestroyType(
       context.specific_interfaces().Get(query_specific_interface_id);
   auto destroy_interface_id = query_specific_interface.interface_id;
 
-  auto inst = context.insts().Get(context.constant_values().GetInstId(
-      GetCanonicalFacetOrTypeValue(context, query_self_const_id)));
+  auto inst_id = context.constant_values().GetInstId(
+      GetCanonicalFacetOrTypeValue(context, query_self_const_id));
+  auto inst = context.insts().Get(inst_id);
+
+  // For facet values, look if the FacetType provides the same.
+  if (auto facet_type =
+          context.types().TryGetAs<SemIR::FacetType>(inst.type_id())) {
+    const auto& info = context.facet_types().Get(facet_type->facet_type_id);
+    for (auto interface : info.extend_constraints) {
+      if (interface.interface_id == destroy_interface_id) {
+        return DestroyFormat::Trivial;
+      }
+    }
+    return DestroyFormat::NoDestroy;
+  }
+
+  // Incomplete types can not be destroyed.
+  auto type_id = context.types().GetTypeIdForTypeInstId(inst_id);
+  if (!TryToCompleteType(context, type_id, loc_id)) {
+    return DestroyFormat::NoDestroy;
+  }
 
   CARBON_KIND_SWITCH(inst) {
     case SemIR::ImplWitnessAccess::Kind:
     case SemIR::SymbolicBinding::Kind: {
-      // These are symbolic, so should never reach `MakeDestroyOpBody`.
-      // For facet values, look if the FacetType provides the same.
-      if (auto facet_type =
-              context.types().TryGetAs<SemIR::FacetType>(inst.type_id())) {
-        const auto& info = context.facet_types().Get(facet_type->facet_type_id);
-        for (auto interface : info.extend_constraints) {
-          if (interface.interface_id == destroy_interface_id) {
-            return DestroyFormat::Trivial;
-          }
-        }
-      }
+      // A symbolic facet of type `type`. Such symbolic values can't be
+      // destroyed.
       return DestroyFormat::NoDestroy;
     }
 
@@ -207,6 +218,7 @@ static auto CanDestroyType(
 
     case CARBON_KIND(SemIR::ClassType class_type): {
       return CanDestroyClass(context, loc_id, class_type,
+                             context.types().GetCompleteTypeInfo(type_id),
                              query_specific_interface_id,
                              /*is_partial=*/false);
     }
@@ -228,6 +240,7 @@ static auto CanDestroyType(
       auto class_type =
           context.insts().GetAs<SemIR::ClassType>(partial_type.inner_id);
       return CanDestroyClass(context, loc_id, class_type,
+                             context.types().GetCompleteTypeInfo(type_id),
                              query_specific_interface_id,
                              /*is_partial=*/true);
     }
@@ -251,12 +264,6 @@ static auto CanDestroyType(
       return has_witness ? DestroyFormat::NonTrivial : DestroyFormat::NoDestroy;
     }
 
-    case CARBON_KIND(SemIR::SymbolicBindingType sym_binding): {
-      return HasWitnessForOneField(context, loc_id,
-                                   sym_binding.facet_value_inst_id,
-                                   query_specific_interface_id);
-    }
-
     case CARBON_KIND(SemIR::TupleType tuple_type): {
       auto block = context.inst_blocks().Get(tuple_type.type_elements_id);
       if (block.empty()) {
@@ -277,7 +284,9 @@ static auto CanDestroyType(
     }
 
     case SemIR::BoolType::Kind:
+    case SemIR::FacetType::Kind:
     case SemIR::FloatType::Kind:
+    case SemIR::IntLiteralType::Kind:
     case SemIR::IntType::Kind:
     case SemIR::PointerType::Kind:
     case SemIR::TypeType::Kind:

toolchain/check/testdata/impl/custom_witness/destroy.carbon

@@ -0,0 +1,79 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/int.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/custom_witness/destroy.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/custom_witness/destroy.carbon
+
+// --- destroy_type.carbon
+library "[[@TEST_NAME]]";
+
+fn F() {
+  // This looks for a Destroy witness for a TypeType.
+  type as Core.Destroy;
+}
+
+// --- destroy_facet_type.carbon
+library "[[@TEST_NAME]]";
+
+interface Z {}
+
+fn F() {
+  // This looks for a Destroy witness for a FacetType.
+  (Z & Core.Destroy) as Core.Destroy;
+}
+
+// --- destroy_int_literal_type.carbon
+library "[[@TEST_NAME]]";
+
+fn F() {
+  // This looks for a Destroy witness for an IntLiteralType.
+  Core.IntLiteral() as Core.Destroy;
+}
+
+// --- destroy_int_type.carbon
+library "[[@TEST_NAME]]";
+
+fn F() {
+  // This looks for a Destroy witness for a class type that adapts IntType, and
+  // has a symbolic CompleteTypeWitness.
+  Core.Int(32) as Core.Destroy;
+}
+
+// --- destroy_symbolic_int_type.carbon
+library "[[@TEST_NAME]]";
+
+fn F(N:! Core.IntLiteral()) {
+  // This looks for a Destroy witness for a class type that adapts IntType with a
+  // symbolic type.
+  Core.Int(N) as Core.Destroy;
+}
+
+// --- destroy_class_type.carbon
+library "[[@TEST_NAME]]";
+
+class C(T:! type) {}
+
+fn F() {
+  // This looks for a Destroy witness for a class type.
+  C({}) as Core.Destroy;
+}
+
+// --- fail_destroy_incomplete_class.carbon
+library "[[@TEST_NAME]]";
+
+class C(T:! type);
+
+fn F() {
+  // This looks for a Destroy witness for an incomplete class type.
+  // CHECK:STDERR: fail_destroy_incomplete_class.carbon:[[@LINE+4]]:3: error: cannot convert type `C({})` into type implementing `Core.Destroy` [ConversionFailureTypeToFacet]
+  // CHECK:STDERR:   C({}) as Core.Destroy;
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  C({}) as Core.Destroy;
+}

toolchain/check/type_completion.cpp

@@ -820,11 +820,11 @@ auto RequireConcreteType(Context& context, SemIR::TypeId type_id,
 
   // TODO: For symbolic types, should add an implicit constraint that they are
   // not abstract.
-  auto complete_info = context.types().GetCompleteTypeInfo(type_id);
+  const auto& complete_info = context.types().GetCompleteTypeInfo(type_id);
   CARBON_CHECK(complete_info.value_repr.type_id.has_value(),
                "RequireConcreteType called for an incomplete type. Call "
                "RequireCompleteType first.");
-  if (!complete_info.abstract_class_id.has_value()) {
+  if (!complete_info.IsAbstract()) {
     return true;
   }
 

toolchain/sem_ir/type_info.h

@@ -83,6 +83,14 @@ struct CompleteTypeInfo : public Printable<CompleteTypeInfo> {
 
   // If this type is abstract, this is id of an abstract class it uses.
   ClassId abstract_class_id = ClassId::None;
+
+  // Returns whether the type is abstract.
+  //
+  // The type must be completed before we can determine if it's abstract.
+  auto IsAbstract() const -> bool {
+    CARBON_CHECK(value_repr.kind != ValueRepr::Unknown);
+    return abstract_class_id.has_value();
+  }
 };
 
 // The representation to use for an initializing expression of some type.