Treat type modifiers as distinct type structure (#6073)

This came up because `const T` needs destructor support... This change
makes `impl T as Destroy` and `impl const T as Destroy` distinct type
structures. Right now there's no impl lookup fallback (see
[#6068](https://github.com/carbon-language/carbon-lang/issues/6068)); so
when trying to destroy `const T`, there's no way to have an `impl` for
it to find.

In discussion, `MaybeUnformed` and `partial` have similar challenges, so
I'm covering them together.

In type_structure.h, I'm switching to an enum because it felt like an
easier way to be adding more types. I can switch back if preferred,
though then might take a closer look at the `operator==` because that's
kind of verbose.

toolchain/check/testdata/impl/lookup/impl_overlap_wrapped_types.carbon

@@ -0,0 +1,114 @@
+// 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/full.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/lookup/impl_overlap_wrapped_types.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/lookup/impl_overlap_wrapped_types.carbon
+
+// --- wrapped_types.carbon
+library "[[@TEST_NAME]]";
+
+base class C {}
+base class D {}
+interface I {}
+
+impl C as I {}
+impl const C as I {}
+impl Core.MaybeUnformed(C) as I {}
+impl partial C as I {}
+impl C* as I {}
+
+impl D as I {}
+impl const D as I {}
+impl Core.MaybeUnformed(D) as I {}
+impl partial D as I {}
+impl D* as I {}
+
+// --- fail_const_overlap.carbon
+library "[[@TEST_NAME]]";
+
+base class C {}
+interface I {}
+
+impl C as I {}
+impl const C as I {}
+impl Core.MaybeUnformed(C) as I {}
+impl partial C as I {}
+impl C* as I {}
+
+// CHECK:STDERR: fail_const_overlap.carbon:[[@LINE+7]]:1: error: redefinition of `impl const C as I` [ImplRedefinition]
+// CHECK:STDERR: impl const C as I {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_const_overlap.carbon:[[@LINE-8]]:1: note: previous definition was here [ImplPreviousDefinition]
+// CHECK:STDERR: impl const C as I {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+impl const C as I {}
+
+// --- fail_maybe_unformed_overlap.carbon
+library "[[@TEST_NAME]]";
+
+base class C {}
+interface I {}
+
+impl C as I {}
+impl const C as I {}
+impl Core.MaybeUnformed(C) as I {}
+impl partial C as I {}
+impl C* as I {}
+
+// CHECK:STDERR: fail_maybe_unformed_overlap.carbon:[[@LINE+7]]:1: error: redefinition of `impl Core.MaybeUnformed(C) as I` [ImplRedefinition]
+// CHECK:STDERR: impl Core.MaybeUnformed(C) as I {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_maybe_unformed_overlap.carbon:[[@LINE-7]]:1: note: previous definition was here [ImplPreviousDefinition]
+// CHECK:STDERR: impl Core.MaybeUnformed(C) as I {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+impl Core.MaybeUnformed(C) as I {}
+
+// --- fail_partial_overlap.carbon
+library "[[@TEST_NAME]]";
+
+base class C {}
+interface I {}
+
+impl C as I {}
+impl const C as I {}
+impl Core.MaybeUnformed(C) as I {}
+impl partial C as I {}
+impl C* as I {}
+
+// CHECK:STDERR: fail_partial_overlap.carbon:[[@LINE+7]]:1: error: redefinition of `impl partial C as I` [ImplRedefinition]
+// CHECK:STDERR: impl partial C as I {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR: fail_partial_overlap.carbon:[[@LINE-6]]:1: note: previous definition was here [ImplPreviousDefinition]
+// CHECK:STDERR: impl partial C as I {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
+impl partial C as I {}
+
+// --- fail_pointer_overlap.carbon
+library "[[@TEST_NAME]]";
+
+base class C {}
+interface I {}
+
+impl C as I {}
+impl const C as I {}
+impl Core.MaybeUnformed(C) as I {}
+impl partial C as I {}
+impl C* as I {}
+
+// CHECK:STDERR: fail_pointer_overlap.carbon:[[@LINE+7]]:1: error: redefinition of `impl C* as I` [ImplRedefinition]
+// CHECK:STDERR: impl C* as I {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~
+// CHECK:STDERR: fail_pointer_overlap.carbon:[[@LINE-5]]:1: note: previous definition was here [ImplPreviousDefinition]
+// CHECK:STDERR: impl C* as I {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~
+// CHECK:STDERR:
+impl C* as I {}

toolchain/check/type_structure.cpp

@@ -255,19 +255,21 @@ auto TypeStructureBuilder::Build(SemIR::TypeIterator type_iter)
         break;
       }
       case CARBON_KIND(Step::StructStartOnly _): {
-        AppendStructuralConcrete(TypeStructure::ConcreteStructType());
+        AppendStructuralConcrete(TypeStructure::ConcreteTypeStart::Struct);
         break;
       }
       case CARBON_KIND(Step::StructStart _): {
-        AppendStructuralConcreteOpenParen(TypeStructure::ConcreteStructType());
+        AppendStructuralConcreteOpenParen(
+            TypeStructure::ConcreteTypeStart::Struct);
         break;
       }
       case CARBON_KIND(Step::TupleStartOnly _): {
-        AppendStructuralConcrete(TypeStructure::ConcreteTupleType());
+        AppendStructuralConcrete(TypeStructure::ConcreteTypeStart::Tuple);
         break;
       }
       case CARBON_KIND(Step::TupleStart _): {
-        AppendStructuralConcreteOpenParen(TypeStructure::ConcreteTupleType());
+        AppendStructuralConcreteOpenParen(
+            TypeStructure::ConcreteTypeStart::Tuple);
         break;
       }
       case CARBON_KIND(Step::InterfaceStartOnly interface_start): {
@@ -282,12 +284,31 @@ auto TypeStructureBuilder::Build(SemIR::TypeIterator type_iter)
         AppendStructuralConcreteOpenParen(int_start.type_id);
         break;
       }
+
+      // Types which only have an `OpenParen` shape.
       case CARBON_KIND(Step::ArrayStart _): {
-        AppendStructuralConcreteOpenParen(TypeStructure::ConcreteArrayType());
+        AppendStructuralConcreteOpenParen(
+            TypeStructure::ConcreteTypeStart::Array);
+        break;
+      }
+      case CARBON_KIND(Step::ConstStart _): {
+        AppendStructuralConcreteOpenParen(
+            TypeStructure::ConcreteTypeStart::Const);
+        break;
+      }
+      case CARBON_KIND(Step::MaybeUnformedStart _): {
+        AppendStructuralConcreteOpenParen(
+            TypeStructure::ConcreteTypeStart::MaybeUnformed);
+        break;
+      }
+      case CARBON_KIND(Step::PartialStart _): {
+        AppendStructuralConcreteOpenParen(
+            TypeStructure::ConcreteTypeStart::Partial);
         break;
       }
       case CARBON_KIND(Step::PointerStart _): {
-        AppendStructuralConcreteOpenParen(TypeStructure::ConcretePointerType());
+        AppendStructuralConcreteOpenParen(
+            TypeStructure::ConcreteTypeStart::Pointer);
         break;
       }
     }

toolchain/check/type_structure.h

@@ -10,6 +10,7 @@
 #include "common/ostream.h"
 #include "llvm/ADT/SmallVector.h"
 #include "toolchain/check/context.h"
+#include "toolchain/check/scope_index.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::Check {
@@ -115,27 +116,30 @@ class TypeStructure : public Printable<TypeStructure> {
     Symbolic,
   };
 
-  // Marks an array type. The type and bound will appear as other entries.
-  struct ConcreteArrayType {
-    friend auto operator==(ConcreteArrayType /*lhs*/, ConcreteArrayType /*rhs*/)
-        -> bool = default;
-  };
-  // Marks a pointer type. The pointee type will appear as another entry.
-  struct ConcretePointerType {
-    friend auto operator==(ConcretePointerType /*lhs*/,
-                           ConcretePointerType /*rhs*/) -> bool = default;
-  };
-  // Marks a struct type. The field names and types will appear as other
-  // entries.
-  struct ConcreteStructType {
-    friend auto operator==(ConcreteStructType /*lhs*/,
-                           ConcreteStructType /*rhs*/) -> bool = default;
-  };
-  // Marks a tuple type. The type members (if any) will appear as other entries.
-  struct ConcreteTupleType {
-    friend auto operator==(ConcreteTupleType /*lhs*/, ConcreteTupleType /*rhs*/)
-        -> bool = default;
+  // Marks a type of the named kind.
+  enum class ConcreteTypeStart {
+    // The type and bound will appear as other entries.
+    Array,
+
+    // The inner type will appear as another entry.
+    Const,
+
+    // The inner type will appear as another entry.
+    MaybeUnformed,
+
+    // The class type will appear as another entry.
+    Partial,
+
+    // The pointee type will appear as another entry.
+    Pointer,
+
+    // The field names and types will appear as other entries.
+    Struct,
+
+    // The type members (if any) will appear as other entries.
+    Tuple,
   };
+
   // The `concrete_types_` tracks the specific concrete type for each
   // `Structural::Concrete` or `Structural::ConcreteOpenParen` in the type
   // structure.
@@ -146,8 +150,7 @@ class TypeStructure : public Printable<TypeStructure> {
   // `NameId` is used strictly for struct fields, as the field names are part of
   // the struct type.
   using ConcreteType =
-      std::variant<ConcreteArrayType, ConcretePointerType, ConcreteStructType,
-                   ConcreteTupleType, SemIR::ClassId, SemIR::ConstantId,
+      std::variant<ConcreteTypeStart, SemIR::ClassId, SemIR::ConstantId,
                    SemIR::InterfaceId, SemIR::NameId, SemIR::TypeId>;
 
   TypeStructure(llvm::SmallVector<Structural> structure,

toolchain/sem_ir/type_iterator.cpp

@@ -122,26 +122,23 @@ auto TypeIterator::Next() -> Step {
         }
       }
       case CARBON_KIND(SemIR::ConstType const_type): {
-        // We don't stop at `const` since it is a modifier; just move to the
-        // inner type.
+        Push(EndType());
         PushInstId(const_type.inner_id);
-        break;
+        return Step::ConstStart();
       }
-      case CARBON_KIND(SemIR::MaybeUnformedType partial_type): {
-        // We don't stop at `MaybeUnformed` since it is a modifier; just move to
-        // the inner type.
-        PushInstId(partial_type.inner_id);
+      case CARBON_KIND(SemIR::ImplWitnessAssociatedConstant assoc): {
+        Push(assoc.type_id);
         break;
       }
+      case CARBON_KIND(SemIR::MaybeUnformedType maybe_unformed_type): {
+        Push(EndType());
+        PushInstId(maybe_unformed_type.inner_id);
+        return Step::MaybeUnformedStart();
+      }
       case CARBON_KIND(SemIR::PartialType partial_type): {
-        // We don't stop at `partial` since it is a modifier; just move to the
-        // inner type.
+        Push(EndType());
         PushInstId(partial_type.inner_id);
-        break;
-      }
-      case CARBON_KIND(SemIR::ImplWitnessAssociatedConstant assoc): {
-        Push(assoc.type_id);
-        break;
+        return Step::PartialStart();
       }
       case CARBON_KIND(SemIR::PointerType pointer_type): {
         Push(EndType());

toolchain/sem_ir/type_iterator.h

@@ -135,7 +135,10 @@ class TypeIterator::Step {
   struct ArrayStart {
     SemIR::TypeId type_id;
   };
-  // Followed by the pointee type.
+  // Simple wrapped types, followed by the inner type.
+  struct ConstStart {};
+  struct MaybeUnformedStart {};
+  struct PartialStart {};
   struct PointerStart {};
 
   // ===========================================================================
@@ -196,7 +199,8 @@ class TypeIterator::Step {
                    SymbolicValue, StructFieldName, ClassStartOnly,
                    StructStartOnly, TupleStartOnly, InterfaceStartOnly,
                    ClassStart, StructStart, TupleStart, InterfaceStart,
-                   IntStart, ArrayStart, PointerStart, End, Done, Error>;
+                   IntStart, ArrayStart, ConstStart, MaybeUnformedStart,
+                   PartialStart, PointerStart, End, Done, Error>;
 
   template <typename T>
   auto Is() const -> bool {