Improve source location in an import error. (#5887)

When attempting to import a definition of a class with virtual bases,
diagnose the point of use instead of the point of definition of the
class.

---------

Co-authored-by: Chandler Carruth <[email protected]>

Author: zygoloid

toolchain/check/import_cpp.cpp

@@ -620,15 +620,8 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id,
 
   // Import bases.
   for (const auto& base : clang_def->bases()) {
-    if (base.isVirtual()) {
-      // TODO: Handle virtual bases. We don't actually know where they go in the
-      // layout. We may also want to use a different size in the layout for
-      // `partial C`, excluding the virtual base. It's also not entirely safe to
-      // just skip over the virtual base, as the type we would construct would
-      // have a misleading size.
-      context.TODO(import_ir_inst_id, "class with virtual bases");
-      return SemIR::ErrorInst::TypeInstId;
-    }
+    CARBON_CHECK(!base.isVirtual(),
+                 "Should not import definition for class with a virtual base");
 
     auto [base_type_inst_id, base_type_id] =
         MapType(context, import_ir_inst_id, base.getType());
@@ -810,6 +803,17 @@ auto ImportCppClassDefinition(Context& context, SemIR::LocId loc_id,
   clang::CXXRecordDecl* clang_def = clang_decl->getDefinition();
   CARBON_CHECK(clang_def, "Complete type has no definition");
 
+  if (clang_def->getNumVBases()) {
+    // TODO: Handle virtual bases. We don't actually know where they go in the
+    // layout. We may also want to use a different size in the layout for
+    // `partial C`, excluding the virtual base. It's also not entirely safe to
+    // just skip over the virtual base, as the type we would construct would
+    // have a misleading size. For now, treat a C++ class with vbases as
+    // incomplete in Carbon.
+    context.TODO(loc_id, "class with virtual bases");
+    return false;
+  }
+
   auto import_ir_inst_id =
       context.insts().GetCanonicalLocId(class_inst_id).import_ir_inst_id();
   BuildClassDefinition(context, import_ir_inst_id, class_id, class_inst_id,

toolchain/check/testdata/interop/cpp/class/base.carbon

@@ -175,11 +175,12 @@ fn Convert(p: Cpp.B*) {
 
 library "[[@TEST_NAME]]";
 
-// CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+2]]:1: in import [InImport]
-// CHECK:STDERR: ./virtual_inheritance.h:3: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
 import Cpp library "virtual_inheritance.h";
 
 fn Convert(p: Cpp.B*) -> Cpp.A* {
+  // CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+14]]:3: error: semantics TODO: `class with virtual bases` [SemanticsTodo]
+  // CHECK:STDERR:   return p;
+  // CHECK:STDERR:   ^~~~~~~~~
   // CHECK:STDERR: fail_todo_use_virtual_inheritance.carbon:[[@LINE+11]]:3: note: while completing C++ class type `Cpp.B` [InCppTypeCompletion]
   // CHECK:STDERR:   return p;
   // CHECK:STDERR:   ^~~~~~~~~