Don't attempt to merge multiple class definitions. (#3947)

Mutating classes after they're complete leads to inconsistencies such as
complete class types that have incomplete class definitions.

Closes #3945

toolchain/check/handle_class.cpp

@@ -68,6 +68,11 @@ static auto MergeClassRedecl(Context& context, SemIRLoc new_loc,
                         .is_extern = prev_is_extern},
                        prev_import_ir_id);
 
+  if (new_is_definition && prev_class.is_defined()) {
+    // Don't attempt to merge multiple definitions.
+    return false;
+  }
+
   // The introducer kind must match the previous declaration.
   // TODO: The rule here is not yet decided. See #3384.
   if (prev_class.inheritance_kind != new_class.inheritance_kind) {

toolchain/check/testdata/class/fail_extend_cycle.carbon

@@ -0,0 +1,83 @@
+// 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
+//
+// AUTOUPDATE
+
+base class A {
+}
+
+base class B {
+  // This ensures that the compiler treats A as complete.
+  extend base: A;
+}
+
+// CHECK:STDERR: fail_extend_cycle.carbon:[[@LINE+7]]:1: ERROR: Redefinition of `class A`.
+// CHECK:STDERR: base class A {
+// CHECK:STDERR: ^~~~~~~~~~~~~~
+// CHECK:STDERR: fail_extend_cycle.carbon:[[@LINE-11]]:1: Previously defined here.
+// CHECK:STDERR: base class A {
+// CHECK:STDERR: ^~~~~~~~~~~~~~
+// CHECK:STDERR:
+base class A {
+  extend base: A;
+  // CHECK:STDERR: fail_extend_cycle.carbon:[[@LINE+3]]:10: ERROR: Name `C` not found.
+  // CHECK:STDERR:   var c: C;
+  // CHECK:STDERR:          ^
+  var c: C;
+}
+
+// CHECK:STDOUT: --- fail_extend_cycle.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %A: type = class_type @A [template]
+// CHECK:STDOUT:   %.1: type = struct_type {} [template]
+// CHECK:STDOUT:   %B: type = class_type @B [template]
+// CHECK:STDOUT:   %.2: type = tuple_type () [template]
+// CHECK:STDOUT:   %.3: type = ptr_type {} [template]
+// CHECK:STDOUT:   %.4: type = unbound_element_type B, A [template]
+// CHECK:STDOUT:   %.5: type = struct_type {.base: A} [template]
+// CHECK:STDOUT:   %.6: type = class_type @.1 [template]
+// CHECK:STDOUT:   %.7: type = unbound_element_type <invalid>, A [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Core = %Core
+// CHECK:STDOUT:     .A = %A.decl
+// CHECK:STDOUT:     .B = %B.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Core: <namespace> = namespace [template] {}
+// CHECK:STDOUT:   %A.decl: type = class_decl @A [template = constants.%A] {}
+// CHECK:STDOUT:   %B.decl: type = class_decl @B [template = constants.%B] {}
+// CHECK:STDOUT:   %.decl: type = class_decl @.1 [template = constants.%.6] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @A {
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = constants.%A
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @B {
+// CHECK:STDOUT:   %A.ref: type = name_ref A, file.%A.decl [template = constants.%A]
+// CHECK:STDOUT:   %.loc12: <unbound element of class B> = base_decl A, element0 [template]
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = constants.%B
+// CHECK:STDOUT:   .base = %.loc12
+// CHECK:STDOUT:   extend name_scope2
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @.1 {
+// CHECK:STDOUT:   %A.ref: type = name_ref A, file.%A.decl [template = constants.%A]
+// CHECK:STDOUT:   %.loc23: <unbound element of class <invalid>> = base_decl A, element0 [template]
+// CHECK:STDOUT:   %C.ref: <error> = name_ref C, <error> [template = <error>]
+// CHECK:STDOUT:   %.loc27: <error> = field_decl c, element1 [template]
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = constants.%.6
+// CHECK:STDOUT:   .base = %.loc23
+// CHECK:STDOUT:   .c = %.loc27
+// CHECK:STDOUT:   extend name_scope2
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/check/testdata/class/fail_import_misuses.carbon

@@ -74,21 +74,22 @@ var a: Incomplete;
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Empty: type = class_type @Empty [template]
 // CHECK:STDOUT:   %.1: type = struct_type {} [template]
+// CHECK:STDOUT:   %.2: type = class_type @.1 [template]
 // CHECK:STDOUT:   %Incomplete: type = class_type @Incomplete [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Empty = %import_ref.1
 // CHECK:STDOUT:     .Incomplete = %import_ref.2
 // CHECK:STDOUT:     .Core = %Core
-// CHECK:STDOUT:     .Empty = %Empty.decl
 // CHECK:STDOUT:     .a = %a
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %import_ref.1: type = import_ref ir1, inst+2, loaded [template = constants.%Empty]
 // CHECK:STDOUT:   %import_ref.2: type = import_ref ir1, inst+5, loaded [template = constants.%Incomplete]
 // CHECK:STDOUT:   %Core: <namespace> = namespace [template] {}
 // CHECK:STDOUT:   %import_ref.3 = import_ref ir1, inst+3, unloaded
-// CHECK:STDOUT:   %Empty.decl: type = class_decl @Empty [template = constants.%Empty] {}
+// CHECK:STDOUT:   %.decl: type = class_decl @.1 [template = constants.%.2] {}
 // CHECK:STDOUT:   %Incomplete.ref: type = name_ref Incomplete, %import_ref.2 [template = constants.%Incomplete]
 // CHECK:STDOUT:   %a.var: ref <error> = var a
 // CHECK:STDOUT:   %a: ref <error> = bind_name a, %a.var
@@ -96,7 +97,12 @@ var a: Incomplete;
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @Empty {
 // CHECK:STDOUT: !members:
-// CHECK:STDOUT:   .Self = constants.%Empty
+// CHECK:STDOUT:   .Self = file.%import_ref.3
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @.1 {
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = constants.%.2
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @Incomplete;

toolchain/check/testdata/class/fail_redefinition.carbon

@@ -23,17 +23,17 @@ class Class {
   fn I() {}
 }
 
+fn Class.F() {}
 // CHECK:STDERR: fail_redefinition.carbon:[[@LINE+4]]:10: ERROR: Out-of-line declaration requires a declaration in scoped entity.
-// CHECK:STDERR: fn Class.F() {}
+// CHECK:STDERR: fn Class.G() {}
 // CHECK:STDERR:          ^
 // CHECK:STDERR:
-fn Class.F() {}
 fn Class.G() {}
 fn Class.H() {}
 // CHECK:STDERR: fail_redefinition.carbon:[[@LINE+6]]:1: ERROR: Redefinition of `fn I`.
 // CHECK:STDERR: fn Class.I() {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~
-// CHECK:STDERR: fail_redefinition.carbon:[[@LINE-13]]:3: Previously defined here.
+// CHECK:STDERR: fail_redefinition.carbon:[[@LINE-26]]:3: Previously defined here.
 // CHECK:STDERR:   fn I() {}
 // CHECK:STDERR:   ^~~~~~~~
 fn Class.I() {}
@@ -42,7 +42,7 @@ fn Class.I() {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Class: type = class_type @Class [template]
-// CHECK:STDOUT:   %F.1: type = fn_type @F.1 [template]
+// CHECK:STDOUT:   %F: type = fn_type @F [template]
 // CHECK:STDOUT:   %.1: type = tuple_type () [template]
 // CHECK:STDOUT:   %struct.1: F = struct_value () [template]
 // CHECK:STDOUT:   %H.1: type = fn_type @H.1 [template]
@@ -50,71 +50,84 @@ fn Class.I() {}
 // CHECK:STDOUT:   %I.1: type = fn_type @I.1 [template]
 // CHECK:STDOUT:   %struct.3: I = struct_value () [template]
 // CHECK:STDOUT:   %.2: type = struct_type {} [template]
-// CHECK:STDOUT:   %G: type = fn_type @G [template]
+// CHECK:STDOUT:   %.3: type = class_type @.1 [template]
+// CHECK:STDOUT:   %G.1: type = fn_type @G.1 [template]
 // CHECK:STDOUT:   %struct.4: G = struct_value () [template]
 // CHECK:STDOUT:   %H.2: type = fn_type @H.2 [template]
 // CHECK:STDOUT:   %struct.5: H = struct_value () [template]
 // CHECK:STDOUT:   %I.2: type = fn_type @I.2 [template]
 // CHECK:STDOUT:   %struct.6: I = struct_value () [template]
-// CHECK:STDOUT:   %F.2: type = fn_type @F.2 [template]
-// CHECK:STDOUT:   %struct.7: F = struct_value () [template]
+// CHECK:STDOUT:   %G.2: type = fn_type @G.2 [template]
+// CHECK:STDOUT:   %struct.7: G = struct_value () [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
 // CHECK:STDOUT:     .Core = %Core
-// CHECK:STDOUT:     .Class = %Class.decl.loc7
+// CHECK:STDOUT:     .Class = %Class.decl
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %Core: <namespace> = namespace [template] {}
-// CHECK:STDOUT:   %Class.decl.loc7: type = class_decl @Class [template = constants.%Class] {}
-// CHECK:STDOUT:   %Class.decl.loc20: type = class_decl @Class [template = constants.%Class] {}
-// CHECK:STDOUT:   %F.decl: F = fn_decl @F.2 [template = constants.%struct.7] {}
-// CHECK:STDOUT:   %G.decl: G = fn_decl @G [template = constants.%struct.4] {}
-// CHECK:STDOUT:   %H.decl: H = fn_decl @H.2 [template = constants.%struct.5] {}
-// CHECK:STDOUT:   %I.decl: I = fn_decl @I.2 [template = constants.%struct.6] {}
+// CHECK:STDOUT:   %Class.decl: type = class_decl @Class [template = constants.%Class] {}
+// CHECK:STDOUT:   %.decl: type = class_decl @.1 [template = constants.%.3] {}
+// CHECK:STDOUT:   %F.decl: F = fn_decl @F [template = constants.%struct.1] {}
+// CHECK:STDOUT:   %G.decl: G = fn_decl @G.2 [template = constants.%struct.7] {}
+// CHECK:STDOUT:   %H.decl: H = fn_decl @H.1 [template = constants.%struct.2] {}
+// CHECK:STDOUT:   %I.decl: I = fn_decl @I.1 [template = constants.%struct.3] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @Class {
-// CHECK:STDOUT:   %G.decl: G = fn_decl @G [template = constants.%struct.4] {}
-// CHECK:STDOUT:   %H.decl: H = fn_decl @H.2 [template = constants.%struct.5] {}
-// CHECK:STDOUT:   %I.decl: I = fn_decl @I.2 [template = constants.%struct.6] {}
+// CHECK:STDOUT:   %F.decl: F = fn_decl @F [template = constants.%struct.1] {}
+// CHECK:STDOUT:   %H.decl: H = fn_decl @H.1 [template = constants.%struct.2] {}
+// CHECK:STDOUT:   %I.decl: I = fn_decl @I.1 [template = constants.%struct.3] {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT:   .Self = constants.%Class
-// CHECK:STDOUT:   .G = %G.decl
+// CHECK:STDOUT:   .F = %F.decl
 // CHECK:STDOUT:   .H = %H.decl
 // CHECK:STDOUT:   .I = %I.decl
-// CHECK:STDOUT:   .F = file.%F.decl
+// CHECK:STDOUT:   .G = file.%G.decl
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F.1();
+// CHECK:STDOUT: class @.1 {
+// CHECK:STDOUT:   %G.decl: G = fn_decl @G.1 [template = constants.%struct.4] {}
+// CHECK:STDOUT:   %H.decl: H = fn_decl @H.2 [template = constants.%struct.5] {}
+// CHECK:STDOUT:   %I.decl: I = fn_decl @I.2 [template = constants.%struct.6] {}
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @H.1();
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = constants.%.3
+// CHECK:STDOUT:   .G = %G.decl
+// CHECK:STDOUT:   .H = %H.decl
+// CHECK:STDOUT:   .I = %I.decl
+// CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @I.1() {
+// CHECK:STDOUT: fn @F() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @G() {
+// CHECK:STDOUT: fn @H.1() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @H.2() {
+// CHECK:STDOUT: fn @I.1() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
+// CHECK:STDOUT:
+// CHECK:STDOUT: !.loc39:
+// CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: fn @G.1();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @H.2();
+// CHECK:STDOUT:
 // CHECK:STDOUT: fn @I.2() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
-// CHECK:STDOUT:
-// CHECK:STDOUT: !.loc39:
-// CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F.2() {
+// CHECK:STDOUT: fn @G.2() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }

toolchain/check/testdata/class/no_prelude/implicit_import.carbon

@@ -181,20 +181,26 @@ class B {}
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %C: type = class_type @C [template]
 // CHECK:STDOUT:   %.1: type = struct_type {} [template]
+// CHECK:STDOUT:   %.2: type = class_type @.1 [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .C = %C.decl
+// CHECK:STDOUT:     .C = %import_ref.1
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %import_ref.1: type = import_ref ir0, inst+1, loaded [template = constants.%C]
 // CHECK:STDOUT:   %import_ref.2 = import_ref ir0, inst+2, unloaded
-// CHECK:STDOUT:   %C.decl: type = class_decl @C [template = constants.%C] {}
+// CHECK:STDOUT:   %.decl: type = class_decl @.1 [template = constants.%.2] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @C {
 // CHECK:STDOUT: !members:
-// CHECK:STDOUT:   .Self = constants.%C
+// CHECK:STDOUT:   .Self = file.%import_ref.2
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @.1 {
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = constants.%.2
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- def_alias.carbon