Error on impl functions prior to extends base

Since impl will need to be checked against the base class's virtual
functions, and on the basis of the information accumulation principle (I
think?).

This does produce an awkward sequence of diagnostics "can't have impl
without a base class" then "can't have base class after an impl". I
guess we'd prefer to only have the latter? Could defer the "impl without
base" until the end of the class to implement that correctly?

Oh, and currently this will only produce one of "base must come before
fields" or "base must come before impls" - I guess it'd be preferable to
produce both?

toolchain/check/handle_class.cpp

@@ -526,6 +526,21 @@ auto HandleParseNode(Context& context, Parse::BaseDeclId node_id) -> bool {
     return true;
   }
 
+  // TODO: base must appear before virtual functions too
+  for (auto inst_id : context.inst_block_stack().PeekCurrentBlockContents()) {
+    if (auto function_decl =
+            context.insts().TryGetAs<SemIR::FunctionDecl>(inst_id)) {
+      auto& function = context.functions().Get(function_decl->function_id);
+      if (function.virtual_modifier == SemIR::Function::VirtualModifier::Impl) {
+        CARBON_DIAGNOSTIC(
+            BaseDeclAfterImplDecl, Error,
+            "`base` declaration must appear before impl function declarations");
+        context.emitter().Emit(node_id, BaseDeclAfterImplDecl);
+        return true;
+      }
+    }
+  }
+
   auto base_info = CheckBaseType(context, base_type_node_id, base_type_expr_id);
 
   // TODO: Should we diagnose if there are already any fields?

toolchain/check/testdata/class/virtual_modifiers.carbon

@@ -79,9 +79,10 @@ class C {
 package FailModifiers;
 
 class C {
-  // CHECK:STDERR: fail_modifiers.carbon:[[@LINE+3]]:3: error: impl without base class [ImplWithoutBase]
+  // CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:3: error: impl without base class [ImplWithoutBase]
   // CHECK:STDERR:   impl fn F();
   // CHECK:STDERR:   ^~~~~~~~~~~~
+  // CHECK:STDERR:
   impl fn F();
 }
 
@@ -118,6 +119,25 @@ class Derived {
   impl fn F();
 }
 
+// --- fail_impl_before_base.carbon
+
+package ImplBeforeBase;
+
+base class Base {
+}
+
+class Derived {
+  // CHECK:STDERR: fail_impl_before_base.carbon:[[@LINE+4]]:3: error: impl without base class [ImplWithoutBase]
+  // CHECK:STDERR:   impl fn F();
+  // CHECK:STDERR:   ^~~~~~~~~~~~
+  // CHECK:STDERR:
+  impl fn F();
+  // CHECK:STDERR: fail_impl_before_base.carbon:[[@LINE+3]]:3: error: `base` declaration must appear before impl function declarations [BaseDeclAfterImplDecl]
+  // CHECK:STDERR:   extend base: Base;
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~
+  extend base: Base;
+}
+
 // CHECK:STDOUT: --- modifiers.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
@@ -695,3 +715,56 @@ class Derived {
 // CHECK:STDOUT:
 // CHECK:STDOUT: impl fn @F();
 // CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_impl_before_base.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %Base: type = class_type @Base [template]
+// CHECK:STDOUT:   %empty_struct_type: type = struct_type {} [template]
+// CHECK:STDOUT:   %complete_type.1: <witness> = complete_type_witness %empty_struct_type [template]
+// CHECK:STDOUT:   %Derived: type = class_type @Derived [template]
+// CHECK:STDOUT:   %F.type: type = fn_type @F [template]
+// CHECK:STDOUT:   %F: %F.type = struct_value () [template]
+// CHECK:STDOUT:   %ptr: type = ptr_type <vtable> [template]
+// CHECK:STDOUT:   %struct_type.vptr: type = struct_type {.<vptr>: %ptr} [template]
+// CHECK:STDOUT:   %complete_type.2: <witness> = complete_type_witness %struct_type.vptr [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/...
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Core = imports.%Core
+// CHECK:STDOUT:     .Base = %Base.decl
+// CHECK:STDOUT:     .Derived = %Derived.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Core.import = import Core
+// CHECK:STDOUT:   %Base.decl: type = class_decl @Base [template = constants.%Base] {} {}
+// CHECK:STDOUT:   %Derived.decl: type = class_decl @Derived [template = constants.%Derived] {} {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Base {
+// CHECK:STDOUT:   %complete_type: <witness> = complete_type_witness %empty_struct_type [template = constants.%complete_type.1]
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = constants.%Base
+// CHECK:STDOUT:   complete_type_witness = %complete_type
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Derived {
+// CHECK:STDOUT:   %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {}
+// CHECK:STDOUT:   %Base.ref: type = name_ref Base, file.%Base.decl [template = constants.%Base]
+// CHECK:STDOUT:   %complete_type: <witness> = complete_type_witness %struct_type.vptr [template = constants.%complete_type.2]
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = constants.%Derived
+// CHECK:STDOUT:   .F = %F.decl
+// CHECK:STDOUT:   complete_type_witness = %complete_type
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: impl fn @F();
+// CHECK:STDOUT:

toolchain/diagnostics/diagnostic_kind.def

@@ -221,6 +221,7 @@ CARBON_DIAGNOSTIC_KIND(BaseDeclRepeated)
 CARBON_DIAGNOSTIC_KIND(BaseIsFinal)
 CARBON_DIAGNOSTIC_KIND(BaseMissingExtend)
 CARBON_DIAGNOSTIC_KIND(BaseDeclAfterFieldDecl)
+CARBON_DIAGNOSTIC_KIND(BaseDeclAfterImplDecl)
 CARBON_DIAGNOSTIC_KIND(ClassAbstractHere)
 CARBON_DIAGNOSTIC_KIND(ClassForwardDeclaredHere)
 CARBON_DIAGNOSTIC_KIND(ClassSpecificDeclOutsideClass)