Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow abstract or base on class declarations (that are not definitions) #4378

Merged
merged 9 commits into from
Oct 10, 2024
Merged

Disallow abstract or base on class declarations (that are not definitions) #4378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1c3c92b
Baseline
dwblaikie Oct 5, 2024
e438053
Disallow `abstract` or `base` on class declarations (that are not def…
dwblaikie Oct 7, 2024
af59051
Remove dead test coverage
dwblaikie Oct 7, 2024
964be4f
Separate out some test cases
dwblaikie Oct 9, 2024
068c7ec
Rephrase the warning for modifiers that would be acceptable on defini…
dwblaikie Oct 9, 2024
854694f
Simplify the implementation of the decl-only-on-def modifier diagnostic
dwblaikie Oct 10, 2024
49a27a7
Add file splitting to some tests
dwblaikie Oct 10, 2024
bf9c0cf
Pull out a common subexpression
dwblaikie Oct 10, 2024
cfc5940
Bit more fixing for tests now that I understand split tests better
dwblaikie Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 7 additions & 15 deletions toolchain/check/handle_class.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
6 changes: 6 additions & 0 deletions toolchain/check/modifiers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
186 changes: 126 additions & 60 deletions toolchain/check/testdata/class/fail_abstract.carbon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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]
Expand All @@ -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 {
Expand All @@ -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] {} {}
Expand All @@ -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:
Loading
Loading