diff --git a/toolchain/check/class.cpp b/toolchain/check/class.cpp index b1ed86cf5fb0..007028d43965 100644 --- a/toolchain/check/class.cpp +++ b/toolchain/check/class.cpp @@ -169,6 +169,7 @@ static auto BuildVtable(Context& context, Parse::ClassDefinitionId node_id, }; llvm::SmallVector vtable; + Set implemented_impls; if (base_vtable_id.has_value()) { auto base_vtable_inst_block = context.inst_blocks().Get( context.vtables().Get(base_vtable_id).virtual_functions_id); @@ -190,8 +191,10 @@ static auto BuildVtable(Context& context, Parse::ClassDefinitionId node_id, override_fn.name_id == fn.name_id; }); if (i != vtable_contents.end()) { - auto& override_fn = context.functions().Get( - context.insts().GetAs(*i).function_id); + auto override_fn_id = + context.insts().GetAs(*i).function_id; + implemented_impls.Insert(override_fn_id); + auto& override_fn = context.functions().Get(override_fn_id); CheckFunctionTypeMatches(context, override_fn, fn, specific_id, /*check_syntax=*/false, /*check_self=*/false); @@ -209,6 +212,10 @@ static auto BuildVtable(Context& context, Parse::ClassDefinitionId node_id, if (fn.virtual_modifier != SemIR::FunctionFields::VirtualModifier::Impl) { fn.virtual_index = vtable.size(); vtable.push_back(build_specific_function(inst_id)); + } else if (!implemented_impls.Lookup(fn_decl.function_id)) { + CARBON_DIAGNOSTIC(ImplWithoutVirtualInBase, Error, + "impl without compatible virtual in base class"); + context.emitter().Emit(SemIR::LocId(inst_id), ImplWithoutVirtualInBase); } } diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index b667f824f36b..f6a38f11b149 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -327,7 +327,7 @@ static auto IsGenericFunction(Context& context, // Requests a vtable be created when processing a virtual function. static auto RequestVtableIfVirtual( Context& context, Parse::AnyFunctionDeclId node_id, - SemIR::Function::VirtualModifier virtual_modifier, + SemIR::Function::VirtualModifier& virtual_modifier, const std::optional& parent_scope_inst, SemIR::InstId decl_id, SemIR::GenericId generic_id) -> void { // In order to request a vtable, the function must be virtual, and in a class @@ -346,11 +346,14 @@ static auto RequestVtableIfVirtual( !class_info.base_id.has_value()) { CARBON_DIAGNOSTIC(ImplWithoutBase, Error, "impl without base class"); context.emitter().Emit(node_id, ImplWithoutBase); + virtual_modifier = SemIR::Function::VirtualModifier::None; + return; } if (IsGenericFunction(context, generic_id, class_info.generic_id)) { CARBON_DIAGNOSTIC(GenericVirtual, Error, "generic virtual function"); context.emitter().Emit(node_id, GenericVirtual); + virtual_modifier = SemIR::Function::VirtualModifier::None; return; } @@ -528,8 +531,8 @@ static auto BuildFunctionDecl(Context& context, // TODO: Validate that the redeclaration doesn't set an access modifier. } - RequestVtableIfVirtual(context, node_id, virtual_modifier, parent_scope_inst, - decl_id, function_info.generic_id); + RequestVtableIfVirtual(context, node_id, function_info.virtual_modifier, + parent_scope_inst, decl_id, function_info.generic_id); // Write the function ID into the FunctionDecl. ReplaceInstBeforeConstantUse(context, decl_id, function_decl); diff --git a/toolchain/check/testdata/class/virtual_modifiers.carbon b/toolchain/check/testdata/class/virtual_modifiers.carbon index 5f08fa80fca0..db3e6cc79c66 100644 --- a/toolchain/check/testdata/class/virtual_modifiers.carbon +++ b/toolchain/check/testdata/class/virtual_modifiers.carbon @@ -130,7 +130,7 @@ fn F() { b1.m2 = 4; } -// --- todo_fail_impl_without_base_declaration.carbon +// --- fail_impl_without_base_declaration.carbon library "[[@TEST_NAME]]"; @@ -139,6 +139,10 @@ base class Base { class Derived { extend base: Base; + // CHECK:STDERR: fail_impl_without_base_declaration.carbon:[[@LINE+4]]:3: error: impl without compatible virtual in base class [ImplWithoutVirtualInBase] + // CHECK:STDERR: impl fn F[self: Self](); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: impl fn F[self: Self](); } @@ -1252,10 +1256,8 @@ var v: Base(T1) = {}; // CHECK:STDOUT: %pattern_type: type = pattern_type %C [concrete] // CHECK:STDOUT: %F.type: type = fn_type @F [concrete] // CHECK:STDOUT: %F: %F.type = struct_value () [concrete] -// CHECK:STDOUT: %ptr: type = ptr_type [concrete] -// CHECK:STDOUT: %C.vtable_ptr: ref %ptr = vtable_ptr @C.vtable [concrete] -// CHECK:STDOUT: %struct_type.vptr: type = struct_type {.: %ptr} [concrete] -// CHECK:STDOUT: %complete_type: = complete_type_witness %struct_type.vptr [concrete] +// CHECK:STDOUT: %empty_struct_type: type = struct_type {} [concrete] +// CHECK:STDOUT: %complete_type: = complete_type_witness %empty_struct_type [concrete] // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: imports { @@ -1283,19 +1285,15 @@ var v: Base(T1) = {}; // CHECK:STDOUT: %Self.ref: type = name_ref Self, constants.%C [concrete = constants.%C] // CHECK:STDOUT: %self: %C = bind_name self, %self.param // CHECK:STDOUT: } -// CHECK:STDOUT: %vtable_ptr: ref %ptr = vtable_ptr @C.vtable [concrete = constants.%C.vtable_ptr] -// CHECK:STDOUT: %struct_type.vptr: type = struct_type {.: %ptr} [concrete = constants.%struct_type.vptr] -// CHECK:STDOUT: %complete_type: = complete_type_witness %struct_type.vptr [concrete = constants.%complete_type] +// CHECK:STDOUT: %empty_struct_type: type = struct_type {} [concrete = constants.%empty_struct_type] +// CHECK:STDOUT: %complete_type: = complete_type_witness %empty_struct_type [concrete = constants.%complete_type] // CHECK:STDOUT: complete_type_witness = %complete_type -// CHECK:STDOUT: vtable_ptr = %vtable_ptr // CHECK:STDOUT: // CHECK:STDOUT: !members: // CHECK:STDOUT: .Self = constants.%C // CHECK:STDOUT: .F = %F.decl // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: vtable @C.vtable {} -// CHECK:STDOUT: // CHECK:STDOUT: impl fn @F(%self.param: %C); // CHECK:STDOUT: // CHECK:STDOUT: --- init_members.carbon @@ -1518,7 +1516,7 @@ var v: Base(T1) = {}; // CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: --- todo_fail_impl_without_base_declaration.carbon +// CHECK:STDOUT: --- fail_impl_without_base_declaration.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { // CHECK:STDOUT: %Base: type = class_type @Base [concrete] diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 7fdaadddadf1..bdf7e04699b0 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -269,6 +269,7 @@ CARBON_DIAGNOSTIC_KIND(ClassSpecificDeclPrevious) CARBON_DIAGNOSTIC_KIND(ClassIncompleteWithinDefinition) CARBON_DIAGNOSTIC_KIND(GenericVirtual) CARBON_DIAGNOSTIC_KIND(ImplWithoutBase) +CARBON_DIAGNOSTIC_KIND(ImplWithoutVirtualInBase) CARBON_DIAGNOSTIC_KIND(VirtualWithoutSelf) // Deduction.