From 14bb9dd5cf935b29fa2ba72a49898a7d22a8a293 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Tue, 26 Nov 2024 17:37:49 -0800 Subject: [PATCH] disallow impl without base (#4583) --- toolchain/check/handle_function.cpp | 5 + .../testdata/class/virtual_modifiers.carbon | 249 ++++++++++++++++-- toolchain/diagnostics/diagnostic_kind.def | 1 + 3 files changed, 233 insertions(+), 22 deletions(-) diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index c7e62ac65db5..1239384857f0 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -212,6 +212,11 @@ static auto BuildFunctionDecl(Context& context, parent_scope_inst) { if (auto class_decl = parent_scope_inst->TryAs()) { auto& class_info = context.classes().Get(class_decl->class_id); + if (virtual_modifier == SemIR::Function::VirtualModifier::Impl && + !class_info.base_id.is_valid()) { + CARBON_DIAGNOSTIC(ImplWithoutBase, Error, "impl without base class"); + context.emitter().Build(node_id, ImplWithoutBase).Emit(); + } // TODO: If this is an `impl` function, check there's a matching base // function that's impl or virtual. class_info.is_dynamic = true; diff --git a/toolchain/check/testdata/class/virtual_modifiers.carbon b/toolchain/check/testdata/class/virtual_modifiers.carbon index 2a2eb3136614..ce119f5b22d7 100644 --- a/toolchain/check/testdata/class/virtual_modifiers.carbon +++ b/toolchain/check/testdata/class/virtual_modifiers.carbon @@ -14,16 +14,12 @@ package Modifiers; base class Base { virtual fn H(); - - impl fn I(); } abstract class Abstract { abstract fn J(); virtual fn K(); - - impl fn L(); } // --- todo_fail_later_base.carbon @@ -47,6 +43,48 @@ fn F() { var v: Modifiers.Base = {}; } +// --- impl_abstract.carbon + +package ImplAbstract; + +abstract class A1 { + virtual fn F(); +} + +abstract class A2 { + extend base: A1; + impl fn F(); +} + +// --- impl_base.carbon + +package ImplBase; + +base class B1 { + virtual fn F(); +} + +base class B2 { + extend base: B1; + impl fn F(); +} + +class C { + extend base: B2; + impl fn F(); +} + +// --- fail_modifiers.carbon + +package FailModifiers; + +class C { + // CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:3: error: impl without base class [ImplWithoutBase] + // CHECK:STDERR: impl fn F(); + // CHECK:STDERR: ^~~~~~~~~~~~ + impl fn F(); +} + // --- init_members.carbon package InitMembers; @@ -86,8 +124,6 @@ class Derived { // CHECK:STDOUT: %Base: type = class_type @Base [template] // CHECK:STDOUT: %H.type: type = fn_type @H [template] // CHECK:STDOUT: %H: %H.type = struct_value () [template] -// CHECK:STDOUT: %I.type: type = fn_type @I [template] -// CHECK:STDOUT: %I: %I.type = struct_value () [template] // CHECK:STDOUT: %.1: type = ptr_type [template] // CHECK:STDOUT: %.2: type = struct_type {.: %.1} [template] // CHECK:STDOUT: %.3: = complete_type_witness %.2 [template] @@ -96,8 +132,6 @@ class Derived { // CHECK:STDOUT: %J: %J.type = struct_value () [template] // CHECK:STDOUT: %K.type: type = fn_type @K [template] // CHECK:STDOUT: %K: %K.type = struct_value () [template] -// CHECK:STDOUT: %L.type: type = fn_type @L [template] -// CHECK:STDOUT: %L: %L.type = struct_value () [template] // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: imports { @@ -120,38 +154,30 @@ class Derived { // CHECK:STDOUT: // CHECK:STDOUT: class @Base { // CHECK:STDOUT: %H.decl: %H.type = fn_decl @H [template = constants.%H] {} {} -// CHECK:STDOUT: %I.decl: %I.type = fn_decl @I [template = constants.%I] {} {} -// CHECK:STDOUT: %.loc8: = complete_type_witness %.2 [template = constants.%.3] +// CHECK:STDOUT: %.loc6: = complete_type_witness %.2 [template = constants.%.3] // CHECK:STDOUT: // CHECK:STDOUT: !members: // CHECK:STDOUT: .Self = constants.%Base // CHECK:STDOUT: .H = %H.decl -// CHECK:STDOUT: .I = %I.decl // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: class @Abstract { // CHECK:STDOUT: %J.decl: %J.type = fn_decl @J [template = constants.%J] {} {} // CHECK:STDOUT: %K.decl: %K.type = fn_decl @K [template = constants.%K] {} {} -// CHECK:STDOUT: %L.decl: %L.type = fn_decl @L [template = constants.%L] {} {} -// CHECK:STDOUT: %.loc16: = complete_type_witness %.2 [template = constants.%.3] +// CHECK:STDOUT: %.loc12: = complete_type_witness %.2 [template = constants.%.3] // CHECK:STDOUT: // CHECK:STDOUT: !members: // CHECK:STDOUT: .Self = constants.%Abstract // CHECK:STDOUT: .J = %J.decl // CHECK:STDOUT: .K = %K.decl -// CHECK:STDOUT: .L = %L.decl // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: virtual fn @H(); // CHECK:STDOUT: -// CHECK:STDOUT: impl fn @I(); -// CHECK:STDOUT: // CHECK:STDOUT: abstract fn @J(); // CHECK:STDOUT: // CHECK:STDOUT: virtual fn @K(); // CHECK:STDOUT: -// CHECK:STDOUT: impl fn @L(); -// CHECK:STDOUT: // CHECK:STDOUT: --- todo_fail_later_base.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { @@ -176,7 +202,6 @@ class Derived { // CHECK:STDOUT: %import_ref.1: type = import_ref Modifiers//default, inst+3, loaded [template = constants.%Base] // CHECK:STDOUT: %import_ref.2 = import_ref Modifiers//default, inst+4, unloaded // CHECK:STDOUT: %import_ref.3 = import_ref Modifiers//default, inst+5, unloaded -// CHECK:STDOUT: %import_ref.4 = import_ref Modifiers//default, inst+9, unloaded // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: file { @@ -208,7 +233,6 @@ class Derived { // CHECK:STDOUT: !members: // CHECK:STDOUT: .Self = imports.%import_ref.2 // CHECK:STDOUT: .H = imports.%import_ref.3 -// CHECK:STDOUT: .I = imports.%import_ref.4 // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: virtual fn @F(); @@ -234,7 +258,6 @@ class Derived { // CHECK:STDOUT: %import_ref.1: type = import_ref Modifiers//default, inst+3, loaded [template = constants.%Base] // CHECK:STDOUT: %import_ref.2 = import_ref Modifiers//default, inst+4, unloaded // CHECK:STDOUT: %import_ref.3 = import_ref Modifiers//default, inst+5, unloaded -// CHECK:STDOUT: %import_ref.4 = import_ref Modifiers//default, inst+9, unloaded // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: file { @@ -252,7 +275,6 @@ class Derived { // CHECK:STDOUT: !members: // CHECK:STDOUT: .Self = imports.%import_ref.2 // CHECK:STDOUT: .H = imports.%import_ref.3 -// CHECK:STDOUT: .I = imports.%import_ref.4 // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: fn @F() { @@ -268,6 +290,189 @@ class Derived { // CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: +// CHECK:STDOUT: --- impl_abstract.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %A1: type = class_type @A1 [template] +// CHECK:STDOUT: %F.type.1: type = fn_type @F.1 [template] +// CHECK:STDOUT: %F.1: %F.type.1 = struct_value () [template] +// CHECK:STDOUT: %.1: type = ptr_type [template] +// CHECK:STDOUT: %.2: type = struct_type {.: %.1} [template] +// CHECK:STDOUT: %.3: = complete_type_witness %.2 [template] +// CHECK:STDOUT: %A2: type = class_type @A2 [template] +// CHECK:STDOUT: %.5: type = unbound_element_type %A2, %A1 [template] +// CHECK:STDOUT: %F.type.2: type = fn_type @F.2 [template] +// CHECK:STDOUT: %F.2: %F.type.2 = struct_value () [template] +// CHECK:STDOUT: %.6: type = struct_type {.base: %A1} [template] +// CHECK:STDOUT: %.7: = complete_type_witness %.6 [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: imports { +// CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { +// CHECK:STDOUT: import Core//prelude +// CHECK:STDOUT: import Core//prelude/... +// CHECK:STDOUT: } +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Core = imports.%Core +// CHECK:STDOUT: .A1 = %A1.decl +// CHECK:STDOUT: .A2 = %A2.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: %Core.import = import Core +// CHECK:STDOUT: %A1.decl: type = class_decl @A1 [template = constants.%A1] {} {} +// CHECK:STDOUT: %A2.decl: type = class_decl @A2 [template = constants.%A2] {} {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @A1 { +// CHECK:STDOUT: %F.decl: %F.type.1 = fn_decl @F.1 [template = constants.%F.1] {} {} +// CHECK:STDOUT: %.loc6: = complete_type_witness %.2 [template = constants.%.3] +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .Self = constants.%A1 +// CHECK:STDOUT: .F = %F.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @A2 { +// CHECK:STDOUT: %A1.ref: type = name_ref A1, file.%A1.decl [template = constants.%A1] +// CHECK:STDOUT: %.loc9: %.5 = base_decl %A1, element0 [template] +// CHECK:STDOUT: %F.decl: %F.type.2 = fn_decl @F.2 [template = constants.%F.2] {} {} +// CHECK:STDOUT: %.loc11: = complete_type_witness %.6 [template = constants.%.7] +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .Self = constants.%A2 +// CHECK:STDOUT: .base = %.loc9 +// CHECK:STDOUT: .F = %F.decl +// CHECK:STDOUT: extend %A1.ref +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: virtual fn @F.1(); +// CHECK:STDOUT: +// CHECK:STDOUT: impl fn @F.2(); +// CHECK:STDOUT: +// CHECK:STDOUT: --- impl_base.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %B1: type = class_type @B1 [template] +// CHECK:STDOUT: %F.type.1: type = fn_type @F.1 [template] +// CHECK:STDOUT: %F.1: %F.type.1 = struct_value () [template] +// CHECK:STDOUT: %.1: type = ptr_type [template] +// CHECK:STDOUT: %.2: type = struct_type {.: %.1} [template] +// CHECK:STDOUT: %.3: = complete_type_witness %.2 [template] +// CHECK:STDOUT: %B2: type = class_type @B2 [template] +// CHECK:STDOUT: %.5: type = unbound_element_type %B2, %B1 [template] +// CHECK:STDOUT: %F.type.2: type = fn_type @F.2 [template] +// CHECK:STDOUT: %F.2: %F.type.2 = struct_value () [template] +// CHECK:STDOUT: %.6: type = struct_type {.base: %B1} [template] +// CHECK:STDOUT: %.7: = complete_type_witness %.6 [template] +// CHECK:STDOUT: %C: type = class_type @C [template] +// CHECK:STDOUT: %.10: type = unbound_element_type %C, %B2 [template] +// CHECK:STDOUT: %F.type.3: type = fn_type @F.3 [template] +// CHECK:STDOUT: %F.3: %F.type.3 = struct_value () [template] +// CHECK:STDOUT: %.11: type = struct_type {.base: %B2} [template] +// CHECK:STDOUT: %.12: = complete_type_witness %.11 [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: imports { +// CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { +// CHECK:STDOUT: import Core//prelude +// CHECK:STDOUT: import Core//prelude/... +// CHECK:STDOUT: } +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Core = imports.%Core +// CHECK:STDOUT: .B1 = %B1.decl +// CHECK:STDOUT: .B2 = %B2.decl +// CHECK:STDOUT: .C = %C.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: %Core.import = import Core +// CHECK:STDOUT: %B1.decl: type = class_decl @B1 [template = constants.%B1] {} {} +// CHECK:STDOUT: %B2.decl: type = class_decl @B2 [template = constants.%B2] {} {} +// CHECK:STDOUT: %C.decl: type = class_decl @C [template = constants.%C] {} {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @B1 { +// CHECK:STDOUT: %F.decl: %F.type.1 = fn_decl @F.1 [template = constants.%F.1] {} {} +// CHECK:STDOUT: %.loc6: = complete_type_witness %.2 [template = constants.%.3] +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .Self = constants.%B1 +// CHECK:STDOUT: .F = %F.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @B2 { +// CHECK:STDOUT: %B1.ref: type = name_ref B1, file.%B1.decl [template = constants.%B1] +// CHECK:STDOUT: %.loc9: %.5 = base_decl %B1, element0 [template] +// CHECK:STDOUT: %F.decl: %F.type.2 = fn_decl @F.2 [template = constants.%F.2] {} {} +// CHECK:STDOUT: %.loc11: = complete_type_witness %.6 [template = constants.%.7] +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .Self = constants.%B2 +// CHECK:STDOUT: .base = %.loc9 +// CHECK:STDOUT: .F = %F.decl +// CHECK:STDOUT: extend %B1.ref +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @C { +// CHECK:STDOUT: %B2.ref: type = name_ref B2, file.%B2.decl [template = constants.%B2] +// CHECK:STDOUT: %.loc14: %.10 = base_decl %B2, element0 [template] +// CHECK:STDOUT: %F.decl: %F.type.3 = fn_decl @F.3 [template = constants.%F.3] {} {} +// CHECK:STDOUT: %.loc16: = complete_type_witness %.11 [template = constants.%.12] +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .Self = constants.%C +// CHECK:STDOUT: .base = %.loc14 +// CHECK:STDOUT: .F = %F.decl +// CHECK:STDOUT: extend %B2.ref +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: virtual fn @F.1(); +// CHECK:STDOUT: +// CHECK:STDOUT: impl fn @F.2(); +// CHECK:STDOUT: +// CHECK:STDOUT: impl fn @F.3(); +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_modifiers.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %C: type = class_type @C [template] +// CHECK:STDOUT: %F.type: type = fn_type @F [template] +// CHECK:STDOUT: %F: %F.type = struct_value () [template] +// CHECK:STDOUT: %.1: type = ptr_type [template] +// CHECK:STDOUT: %.2: type = struct_type {.: %.1} [template] +// CHECK:STDOUT: %.3: = complete_type_witness %.2 [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: imports { +// CHECK:STDOUT: %Core: = namespace file.%Core.import, [template] { +// CHECK:STDOUT: import Core//prelude +// CHECK:STDOUT: import Core//prelude/... +// CHECK:STDOUT: } +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Core = imports.%Core +// CHECK:STDOUT: .C = %C.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: %Core.import = import Core +// CHECK:STDOUT: %C.decl: type = class_decl @C [template = constants.%C] {} {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @C { +// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {} +// CHECK:STDOUT: %.loc9: = complete_type_witness %.2 [template = constants.%.3] +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .Self = constants.%C +// CHECK:STDOUT: .F = %F.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: impl fn @F(); +// CHECK:STDOUT: // CHECK:STDOUT: --- init_members.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index fee0682b3eb4..0c1ca5e674cd 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -225,6 +225,7 @@ CARBON_DIAGNOSTIC_KIND(ClassForwardDeclaredHere) CARBON_DIAGNOSTIC_KIND(ClassSpecificDeclOutsideClass) CARBON_DIAGNOSTIC_KIND(ClassSpecificDeclPrevious) CARBON_DIAGNOSTIC_KIND(ClassIncompleteWithinDefinition) +CARBON_DIAGNOSTIC_KIND(ImplWithoutBase) // Deduction. CARBON_DIAGNOSTIC_KIND(DeductionIncomplete)