Disallow abstract or base on class declarations (that are not def…

…initions) (#4378) Per: [p3762](https://docs.carbon-lang.dev/proposals/p3762.html#modifier-keywords:~:text=Other%20class%2C%20impl%2C%20and%20interface%20modifiers%20%28abstract%2C%20base%2C%20final%29%20exist%20only%20on%20the%20definition%2C%20not%20on%20the%20forward%20declaration): "Other class, impl, and interface modifiers (`abstract`, `base`, `final`) exist only on the definition, not on the forward declaration."
carbon-language · Oct 10, 2024 · b1014bf · b1014bf
1 parent 1338f9e
commit b1014bf
Show file tree

Hide file tree

Showing 8 changed files with 1,178 additions and 935 deletions.
diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp
@@ -79,19 +79,6 @@ static auto MergeClassRedecl(Context& context, SemIRLoc new_loc,
     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) {
-    CARBON_DIAGNOSTIC(ClassRedeclarationDifferentIntroducer, Error,
-                      "class redeclared with different inheritance kind");
-    CARBON_DIAGNOSTIC(ClassRedeclarationDifferentIntroducerPrevious, Note,
-                      "previously declared here");
-    context.emitter()
-        .Build(new_loc, ClassRedeclarationDifferentIntroducer)
-        .Note(prev_loc, ClassRedeclarationDifferentIntroducerPrevious)
-        .Emit();
-  }
-
   if (new_is_definition) {
     prev_class.MergeDefinition(new_class);
     prev_class.scope_id = new_class.scope_id;
@@ -194,9 +181,14 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id,
   auto introducer =
       context.decl_introducer_state_stack().Pop<Lex::TokenKind::Class>();
   CheckAccessModifiersOnDecl(context, introducer, parent_scope_inst);
+  auto always_acceptable_modifiers =
+      KeywordModifierSet::Access | KeywordModifierSet::Extern;
   LimitModifiersOnDecl(context, introducer,
-                       KeywordModifierSet::Class | KeywordModifierSet::Access |
-                           KeywordModifierSet::Extern);
+                       always_acceptable_modifiers | KeywordModifierSet::Class);
+  if (!is_definition) {
+    LimitModifiersOnNotDefinition(context, introducer,
+                                  always_acceptable_modifiers);
+  }
   RestrictExternModifierOnDecl(context, introducer, parent_scope_inst,
                                is_definition);
 

diff --git a/toolchain/check/modifiers.h b/toolchain/check/modifiers.h
@@ -46,6 +46,12 @@ inline auto LimitModifiersOnDecl(Context& context,
   ForbidModifiersOnDecl(context, introducer, ~allowed, "");
 }
 
+inline auto LimitModifiersOnNotDefinition(Context& context,
+                                          DeclIntroducerState& introducer,
+                                          KeywordModifierSet allowed) -> void {
+  ForbidModifiersOnDecl(context, introducer, ~allowed, ", only definition");
+}
+
 // Restricts the `extern` modifier to only be used on namespace-scoped
 // declarations. Diagnoses and cleans up:
 // - `extern library` on a definition.

diff --git a/toolchain/check/testdata/class/fail_abstract.carbon b/toolchain/check/testdata/class/fail_abstract.carbon
@@ -8,6 +8,8 @@
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/class/fail_abstract.carbon
 
+
+// --- fail_todo_rejects_valid_abstract_subobject_construction.carbon
 abstract class Abstract {
   var a: i32;
 }
@@ -20,17 +22,34 @@ class Derived {
 
 fn Make() -> Derived {
   // TODO: This should be valid, and should construct an instance of `partial Abstract` as the base.
-  // CHECK:STDERR: fail_abstract.carbon:[[@LINE+3]]:19: error: cannot construct instance of abstract class; consider using `partial Abstract` instead
+  // CHECK:STDERR: fail_todo_rejects_valid_abstract_subobject_construction.carbon:[[@LINE+6]]:19: error: cannot construct instance of abstract class; consider using `partial Abstract` instead
   // CHECK:STDERR:   return {.base = {.a = 1}, .d = 7};
   // CHECK:STDERR:                   ^~~~~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_todo_rejects_valid_abstract_subobject_access.carbon: error: `Main//default` previously provided by `fail_todo_rejects_valid_abstract_subobject_construction.carbon`
+  // CHECK:STDERR:
   return {.base = {.a = 1}, .d = 7};
 }
 
+// --- fail_todo_rejects_valid_abstract_subobject_access.carbon
+
+// CHECK:STDERR: fail_todo_rejects_valid_abstract_subobject_access.carbon:[[@LINE+6]]:14: error: name `Derived` not found
+// CHECK:STDERR: fn Access(d: Derived) -> (i32, i32) {
+// CHECK:STDERR:              ^~~~~~~
+// CHECK:STDERR:
+// CHECK:STDERR: fail_abstract_decl.carbon: error: `Main//default` previously provided by `fail_todo_rejects_valid_abstract_subobject_construction.carbon`
+// CHECK:STDERR:
 fn Access(d: Derived) -> (i32, i32) {
   return (d.d, d.base.a);
 }
 
-// CHECK:STDOUT: --- fail_abstract.carbon
+// --- fail_abstract_decl.carbon
+// CHECK:STDERR: fail_abstract_decl.carbon:[[@LINE+3]]:1: error: `abstract` not allowed on `class` declaration, only definition
+// CHECK:STDERR: abstract class AbstractDecl;
+// CHECK:STDERR: ^~~~~~~~
+abstract class AbstractDecl;
+
+// CHECK:STDOUT: --- fail_todo_rejects_valid_abstract_subobject_construction.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %Abstract: type = class_type @Abstract [template]
@@ -54,11 +73,6 @@ fn Access(d: Derived) -> (i32, i32) {
 // CHECK:STDOUT:   %.13: i32 = int_literal 1 [template]
 // CHECK:STDOUT:   %.14: i32 = int_literal 7 [template]
 // CHECK:STDOUT:   %.15: type = struct_type {.base: %.3, .d: i32} [template]
-// CHECK:STDOUT:   %.16: type = tuple_type (type, type) [template]
-// CHECK:STDOUT:   %.17: type = tuple_type (i32, i32) [template]
-// CHECK:STDOUT:   %Access.type: type = fn_type @Access [template]
-// CHECK:STDOUT:   %Access: %Access.type = struct_value () [template]
-// CHECK:STDOUT:   %.18: type = ptr_type %.17 [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -82,7 +96,6 @@ fn Access(d: Derived) -> (i32, i32) {
 // CHECK:STDOUT:     .Abstract = %Abstract.decl
 // CHECK:STDOUT:     .Derived = %Derived.decl
 // CHECK:STDOUT:     .Make = %Make.decl
-// CHECK:STDOUT:     .Access = %Access.decl
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %Core.import = import Core
 // CHECK:STDOUT:   %Abstract.decl: type = class_decl @Abstract [template = constants.%Abstract] {} {}
@@ -91,83 +104,136 @@ fn Access(d: Derived) -> (i32, i32) {
 // CHECK:STDOUT:     %Derived.ref: type = name_ref Derived, file.%Derived.decl [template = constants.%Derived]
 // CHECK:STDOUT:     %return: ref %Derived = var <return slot>
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %Access.decl: %Access.type = fn_decl @Access [template = constants.%Access] {
-// CHECK:STDOUT:     %d.patt: %Derived = binding_pattern d
-// CHECK:STDOUT:   } {
-// CHECK:STDOUT:     %Derived.ref: type = name_ref Derived, file.%Derived.decl [template = constants.%Derived]
-// CHECK:STDOUT:     %d.param: %Derived = param d, runtime_param0
-// CHECK:STDOUT:     %d: %Derived = bind_name d, %d.param
-// CHECK:STDOUT:     %int.make_type_32.loc29_27: init type = call constants.%Int32() [template = i32]
-// CHECK:STDOUT:     %int.make_type_32.loc29_32: init type = call constants.%Int32() [template = i32]
-// CHECK:STDOUT:     %.loc29_35.1: %.16 = tuple_literal (%int.make_type_32.loc29_27, %int.make_type_32.loc29_32)
-// CHECK:STDOUT:     %.loc29_35.2: type = value_of_initializer %int.make_type_32.loc29_27 [template = i32]
-// CHECK:STDOUT:     %.loc29_35.3: type = converted %int.make_type_32.loc29_27, %.loc29_35.2 [template = i32]
-// CHECK:STDOUT:     %.loc29_35.4: type = value_of_initializer %int.make_type_32.loc29_32 [template = i32]
-// CHECK:STDOUT:     %.loc29_35.5: type = converted %int.make_type_32.loc29_32, %.loc29_35.4 [template = i32]
-// CHECK:STDOUT:     %.loc29_35.6: type = converted %.loc29_35.1, constants.%.17 [template = constants.%.17]
-// CHECK:STDOUT:     %return: ref %.17 = var <return slot>
-// CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @Abstract {
 // CHECK:STDOUT:   %int.make_type_32: init type = call constants.%Int32() [template = i32]
-// CHECK:STDOUT:   %.loc12_10.1: type = value_of_initializer %int.make_type_32 [template = i32]
-// CHECK:STDOUT:   %.loc12_10.2: type = converted %int.make_type_32, %.loc12_10.1 [template = i32]
-// CHECK:STDOUT:   %.loc12_8: %.2 = field_decl a, element0 [template]
-// CHECK:STDOUT:   %.loc13: <witness> = complete_type_witness %.3 [template = constants.%.4]
+// CHECK:STDOUT:   %.loc2_10.1: type = value_of_initializer %int.make_type_32 [template = i32]
+// CHECK:STDOUT:   %.loc2_10.2: type = converted %int.make_type_32, %.loc2_10.1 [template = i32]
+// CHECK:STDOUT:   %.loc2_8: %.2 = field_decl a, element0 [template]
+// CHECK:STDOUT:   %.loc3: <witness> = complete_type_witness %.3 [template = constants.%.4]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT:   .Self = constants.%Abstract
-// CHECK:STDOUT:   .a = %.loc12_8
+// CHECK:STDOUT:   .a = %.loc2_8
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @Derived {
 // CHECK:STDOUT:   %Abstract.ref: type = name_ref Abstract, file.%Abstract.decl [template = constants.%Abstract]
-// CHECK:STDOUT:   %.loc16: %.6 = base_decl %Abstract, element0 [template]
+// CHECK:STDOUT:   %.loc6: %.6 = base_decl %Abstract, element0 [template]
 // CHECK:STDOUT:   %int.make_type_32: init type = call constants.%Int32() [template = i32]
-// CHECK:STDOUT:   %.loc18_10.1: type = value_of_initializer %int.make_type_32 [template = i32]
-// CHECK:STDOUT:   %.loc18_10.2: type = converted %int.make_type_32, %.loc18_10.1 [template = i32]
-// CHECK:STDOUT:   %.loc18_8: %.7 = field_decl d, element1 [template]
-// CHECK:STDOUT:   %.loc19: <witness> = complete_type_witness %.8 [template = constants.%.9]
+// CHECK:STDOUT:   %.loc8_10.1: type = value_of_initializer %int.make_type_32 [template = i32]
+// CHECK:STDOUT:   %.loc8_10.2: type = converted %int.make_type_32, %.loc8_10.1 [template = i32]
+// CHECK:STDOUT:   %.loc8_8: %.7 = field_decl d, element1 [template]
+// CHECK:STDOUT:   %.loc9: <witness> = complete_type_witness %.8 [template = constants.%.9]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT:   .Self = constants.%Derived
-// CHECK:STDOUT:   .base = %.loc16
-// CHECK:STDOUT:   .d = %.loc18_8
+// CHECK:STDOUT:   .base = %.loc6
+// CHECK:STDOUT:   .d = %.loc8_8
 // CHECK:STDOUT:   extend name_scope2
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @Int32() -> type = "int.make_type_32";
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @Make() -> %return: %Derived {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %.loc26_25: i32 = int_literal 1 [template = constants.%.13]
-// CHECK:STDOUT:   %.loc26_26: %.3 = struct_literal (%.loc26_25)
-// CHECK:STDOUT:   %.loc26_34: i32 = int_literal 7 [template = constants.%.14]
-// CHECK:STDOUT:   %.loc26_35: %.15 = struct_literal (%.loc26_26, %.loc26_34)
+// CHECK:STDOUT:   %.loc19_25: i32 = int_literal 1 [template = constants.%.13]
+// CHECK:STDOUT:   %.loc19_26: %.3 = struct_literal (%.loc19_25)
+// CHECK:STDOUT:   %.loc19_34: i32 = int_literal 7 [template = constants.%.14]
+// CHECK:STDOUT:   %.loc19_35: %.15 = struct_literal (%.loc19_26, %.loc19_34)
 // CHECK:STDOUT:   return <error> to %return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @Access(%d: %Derived) -> %return: %.17 {
+// CHECK:STDOUT: --- fail_todo_rejects_valid_abstract_subobject_access.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %Int32.type: type = fn_type @Int32 [template]
+// CHECK:STDOUT:   %.1: type = tuple_type () [template]
+// CHECK:STDOUT:   %Int32: %Int32.type = struct_value () [template]
+// CHECK:STDOUT:   %.2: type = tuple_type (type, type) [template]
+// CHECK:STDOUT:   %.3: type = tuple_type (i32, i32) [template]
+// CHECK:STDOUT:   %Access.type: type = fn_type @Access [template]
+// CHECK:STDOUT:   %Access: %Access.type = struct_value () [template]
+// CHECK:STDOUT:   %.4: type = ptr_type %.3 [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// CHECK:STDOUT:   %Core: <namespace> = namespace file.%Core.import, [template] {
+// CHECK:STDOUT:     .Int32 = %import_ref
+// CHECK:STDOUT:     import Core//prelude
+// CHECK:STDOUT:     import Core//prelude/operators
+// CHECK:STDOUT:     import Core//prelude/types
+// CHECK:STDOUT:     import Core//prelude/operators/arithmetic
+// CHECK:STDOUT:     import Core//prelude/operators/as
+// CHECK:STDOUT:     import Core//prelude/operators/bitwise
+// CHECK:STDOUT:     import Core//prelude/operators/comparison
+// CHECK:STDOUT:     import Core//prelude/types/bool
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %import_ref: %Int32.type = import_ref Core//prelude/types, inst+4, loaded [template = constants.%Int32]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Core = imports.%Core
+// CHECK:STDOUT:     .Access = %Access.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Core.import = import Core
+// CHECK:STDOUT:   %Access.decl: %Access.type = fn_decl @Access [template = constants.%Access] {
+// CHECK:STDOUT:     %d.patt: <error> = binding_pattern d
+// CHECK:STDOUT:   } {
+// CHECK:STDOUT:     %Derived.ref: <error> = name_ref Derived, <error> [template = <error>]
+// CHECK:STDOUT:     %d.param: <error> = param d, runtime_param0
+// CHECK:STDOUT:     %d: <error> = bind_name d, %d.param
+// CHECK:STDOUT:     %int.make_type_32.loc8_27: init type = call constants.%Int32() [template = i32]
+// CHECK:STDOUT:     %int.make_type_32.loc8_32: init type = call constants.%Int32() [template = i32]
+// CHECK:STDOUT:     %.loc8_35.1: %.2 = tuple_literal (%int.make_type_32.loc8_27, %int.make_type_32.loc8_32)
+// CHECK:STDOUT:     %.loc8_35.2: type = value_of_initializer %int.make_type_32.loc8_27 [template = i32]
+// CHECK:STDOUT:     %.loc8_35.3: type = converted %int.make_type_32.loc8_27, %.loc8_35.2 [template = i32]
+// CHECK:STDOUT:     %.loc8_35.4: type = value_of_initializer %int.make_type_32.loc8_32 [template = i32]
+// CHECK:STDOUT:     %.loc8_35.5: type = converted %int.make_type_32.loc8_32, %.loc8_35.4 [template = i32]
+// CHECK:STDOUT:     %.loc8_35.6: type = converted %.loc8_35.1, constants.%.3 [template = constants.%.3]
+// CHECK:STDOUT:     %return: ref %.3 = var <return slot>
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Int32() -> type = "int.make_type_32";
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Access(%d: <error>) -> %return: %.3 {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %d.ref.loc30_11: %Derived = name_ref d, %d
-// CHECK:STDOUT:   %d.ref.loc30_12: %.7 = name_ref d, @Derived.%.loc18_8 [template = @Derived.%.loc18_8]
-// CHECK:STDOUT:   %.loc30_12.1: ref i32 = class_element_access %d.ref.loc30_11, element1
-// CHECK:STDOUT:   %.loc30_12.2: i32 = bind_value %.loc30_12.1
-// CHECK:STDOUT:   %d.ref.loc30_16: %Derived = name_ref d, %d
-// CHECK:STDOUT:   %base.ref: %.6 = name_ref base, @Derived.%.loc16 [template = @Derived.%.loc16]
-// CHECK:STDOUT:   %.loc30_17.1: ref %Abstract = class_element_access %d.ref.loc30_16, element0
-// CHECK:STDOUT:   %.loc30_17.2: %Abstract = bind_value %.loc30_17.1
-// CHECK:STDOUT:   %a.ref: %.2 = name_ref a, @Abstract.%.loc12_8 [template = @Abstract.%.loc12_8]
-// CHECK:STDOUT:   %.loc30_22.1: ref i32 = class_element_access %.loc30_17.2, element0
-// CHECK:STDOUT:   %.loc30_22.2: i32 = bind_value %.loc30_22.1
-// CHECK:STDOUT:   %.loc30_24.1: %.17 = tuple_literal (%.loc30_12.2, %.loc30_22.2)
-// CHECK:STDOUT:   %.loc30_24.2: ref i32 = tuple_access %return, element0
-// CHECK:STDOUT:   %.loc30_24.3: init i32 = initialize_from %.loc30_12.2 to %.loc30_24.2
-// CHECK:STDOUT:   %.loc30_24.4: ref i32 = tuple_access %return, element1
-// CHECK:STDOUT:   %.loc30_24.5: init i32 = initialize_from %.loc30_22.2 to %.loc30_24.4
-// CHECK:STDOUT:   %.loc30_24.6: init %.17 = tuple_init (%.loc30_24.3, %.loc30_24.5) to %return
-// CHECK:STDOUT:   %.loc30_25: init %.17 = converted %.loc30_24.1, %.loc30_24.6
-// CHECK:STDOUT:   return %.loc30_25 to %return
+// CHECK:STDOUT:   %d.ref.loc9_11: <error> = name_ref d, %d
+// CHECK:STDOUT:   %d.ref.loc9_16: <error> = name_ref d, %d
+// CHECK:STDOUT:   %.loc9: <error> = tuple_literal (<error>, <error>)
+// CHECK:STDOUT:   return <error> to %return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_abstract_decl.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %AbstractDecl: type = class_type @AbstractDecl [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: imports {
+// CHECK:STDOUT:   %Core: <namespace> = namespace file.%Core.import, [template] {
+// CHECK:STDOUT:     import Core//prelude
+// CHECK:STDOUT:     import Core//prelude/operators
+// CHECK:STDOUT:     import Core//prelude/types
+// CHECK:STDOUT:     import Core//prelude/operators/arithmetic
+// CHECK:STDOUT:     import Core//prelude/operators/as
+// CHECK:STDOUT:     import Core//prelude/operators/bitwise
+// CHECK:STDOUT:     import Core//prelude/operators/comparison
+// CHECK:STDOUT:     import Core//prelude/types/bool
+// CHECK:STDOUT:   }
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Core = imports.%Core
+// CHECK:STDOUT:     .AbstractDecl = %AbstractDecl.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Core.import = import Core
+// CHECK:STDOUT:   %AbstractDecl.decl: type = class_decl @AbstractDecl [template = constants.%AbstractDecl] {} {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @AbstractDecl;
+// CHECK:STDOUT: