When adding an imported C++ name, make sure that its `clang::Decl` is mapped if import failed (#5769)

When mapping parameter types, we assume that if the `clang::Decl` isn't
mapped, the name wasn't added, so this fixes a bug that triggers a crash
otherwise.

Part of #5533.

toolchain/check/import_cpp.cpp

@@ -453,6 +453,12 @@ static auto BuildClassDefinition(Context& context,
   return {class_id, class_inst_id};
 }
 
+// Mark the given `Decl` as failed in `clang_decls`.
+static auto MarkFailedDecl(Context& context, clang::Decl* clang_decl) {
+  context.sem_ir().clang_decls().Add(
+      {.decl = clang_decl, .inst_id = SemIR::ErrorInst::InstId});
+}
+
 // Imports a record declaration from Clang to Carbon. If successful, returns
 // the new Carbon class declaration `InstId`.
 // TODO: Change `clang_decl` to `const &` when lookup is using `clang::DeclID`
@@ -466,11 +472,13 @@ static auto ImportCXXRecordDecl(Context& context, SemIR::LocId loc_id,
   if (!clang_def) {
     context.TODO(loc_id,
                  "Unsupported: Record declarations without a definition");
+    MarkFailedDecl(context, clang_decl);
     return SemIR::ErrorInst::InstId;
   }
 
   if (clang_def->isDynamicClass()) {
     context.TODO(loc_id, "Unsupported: Dynamic Class");
+    MarkFailedDecl(context, clang_decl);
     return SemIR::ErrorInst::InstId;
   }
 
@@ -717,14 +725,17 @@ static auto ImportFunctionDecl(Context& context, SemIR::LocId loc_id,
     -> SemIR::InstId {
   if (clang_decl->isVariadic()) {
     context.TODO(loc_id, "Unsupported: Variadic function");
+    MarkFailedDecl(context, clang_decl);
     return SemIR::ErrorInst::InstId;
   }
   if (!clang_decl->isGlobal()) {
     context.TODO(loc_id, "Unsupported: Non-global function");
+    MarkFailedDecl(context, clang_decl);
     return SemIR::ErrorInst::InstId;
   }
   if (clang_decl->getTemplatedKind() != clang::FunctionDecl::TK_NonTemplate) {
     context.TODO(loc_id, "Unsupported: Template function");
+    MarkFailedDecl(context, clang_decl);
     return SemIR::ErrorInst::InstId;
   }
 
@@ -740,6 +751,7 @@ static auto ImportFunctionDecl(Context& context, SemIR::LocId loc_id,
   context.scope_stack().Pop();
 
   if (!function_params_insts.has_value()) {
+    MarkFailedDecl(context, clang_decl);
     return SemIR::ErrorInst::InstId;
   }
 

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

@@ -44,6 +44,78 @@ fn F() {
   Cpp.foo({});
 }
 
+// --- fail_todo_import_decl_value_param_type_previously_imported.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "decl_value_param_type.h";
+
+fn F() {
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+11]]:10: error: semantics TODO: `Unsupported: Record declarations without a definition` [SemanticsTodo]
+  // CHECK:STDERR:   let c: Cpp.C;
+  // CHECK:STDERR:          ^~~~~
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+8]]:10: note: in `Cpp` name lookup for `C` [InCppNameLookup]
+  // CHECK:STDERR:   let c: Cpp.C;
+  // CHECK:STDERR:          ^~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+4]]:15: error: expected `=`; `let` declaration must have an initializer [ExpectedInitializerAfterLet]
+  // CHECK:STDERR:   let c: Cpp.C;
+  // CHECK:STDERR:               ^
+  // CHECK:STDERR:
+  let c: Cpp.C;
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: parameter type: class C` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo(c);
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo(c);
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR:
+  Cpp.foo(c);
+}
+
+// ============================================================================
+// Forward-declared class as parameter type imported twice
+// ============================================================================
+
+// --- double_decl_value_param_type.h
+
+class C;
+
+auto foo1(C) -> void;
+auto foo2(C) -> void;
+
+// --- fail_todo_import_double_decl_value_param_type.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "double_decl_value_param_type.h";
+
+fn F() {
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+14]]:3: error: semantics TODO: `Unsupported: Record declarations without a definition` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+11]]:3: note: in `Cpp` name lookup for `foo1` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: parameter type: class C` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo1` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR:
+  Cpp.foo1({});
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: parameter type: class C` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo2({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo2` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo2({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR:
+  Cpp.foo2({});
+}
+
 // ============================================================================
 // Defined class without data members as parameter type
 // ============================================================================

toolchain/check/testdata/interop/cpp/function/struct.carbon

@@ -44,6 +44,78 @@ fn F() {
   Cpp.foo({});
 }
 
+// --- fail_todo_import_decl_value_param_type_previously_imported.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "decl_value_param_type.h";
+
+fn F() {
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+11]]:10: error: semantics TODO: `Unsupported: Record declarations without a definition` [SemanticsTodo]
+  // CHECK:STDERR:   let s: Cpp.S;
+  // CHECK:STDERR:          ^~~~~
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+8]]:10: note: in `Cpp` name lookup for `S` [InCppNameLookup]
+  // CHECK:STDERR:   let s: Cpp.S;
+  // CHECK:STDERR:          ^~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+4]]:15: error: expected `=`; `let` declaration must have an initializer [ExpectedInitializerAfterLet]
+  // CHECK:STDERR:   let s: Cpp.S;
+  // CHECK:STDERR:               ^
+  // CHECK:STDERR:
+  let s: Cpp.S;
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: parameter type: struct S` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo(s);
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo(s);
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR:
+  Cpp.foo(s);
+}
+
+// ============================================================================
+// Forward-declared struct as parameter type imported twice
+// ============================================================================
+
+// --- double_decl_value_param_type.h
+
+struct S;
+
+auto foo1(S) -> void;
+auto foo2(S) -> void;
+
+// --- fail_todo_import_double_decl_value_param_type.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "double_decl_value_param_type.h";
+
+fn F() {
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+14]]:3: error: semantics TODO: `Unsupported: Record declarations without a definition` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+11]]:3: note: in `Cpp` name lookup for `foo1` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: parameter type: struct S` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo1` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR:
+  Cpp.foo1({});
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: parameter type: struct S` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo2({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo2` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo2({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR:
+  Cpp.foo2({});
+}
+
 // ============================================================================
 // Defined struct without data members as parameter type
 // ============================================================================

toolchain/check/testdata/interop/cpp/function/union.carbon

@@ -40,6 +40,71 @@ fn F() {
   //@dump-sem-ir-end
 }
 
+// --- fail_todo_import_decl_value_param_type_previously_imported.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "decl_value_param_type.h";
+
+fn F() {
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+11]]:10: error: semantics TODO: `Unsupported: Record declarations without a definition` [SemanticsTodo]
+  // CHECK:STDERR:   let u: Cpp.U;
+  // CHECK:STDERR:          ^~~~~
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+8]]:10: note: in `Cpp` name lookup for `U` [InCppNameLookup]
+  // CHECK:STDERR:   let u: Cpp.U;
+  // CHECK:STDERR:          ^~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+4]]:15: error: expected `=`; `let` declaration must have an initializer [ExpectedInitializerAfterLet]
+  // CHECK:STDERR:   let u: Cpp.U;
+  // CHECK:STDERR:               ^
+  // CHECK:STDERR:
+  let u: Cpp.U;
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: parameter type: union U` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo(u);
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR: fail_todo_import_decl_value_param_type_previously_imported.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo(u);
+  // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR:
+  Cpp.foo(u);
+}
+
+// ============================================================================
+// Forward-declared union as parameter type imported twice
+// ============================================================================
+
+// --- double_decl_value_param_type.h
+
+union U;
+
+auto foo1(U) -> void;
+auto foo2(U) -> void;
+
+// --- fail_todo_import_double_decl_value_param_type.carbon
+
+library "[[@TEST_NAME]]";
+
+import Cpp library "double_decl_value_param_type.h";
+
+fn F() {
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: parameter type: union U` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo1` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo1({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR:
+  Cpp.foo1({});
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+7]]:3: error: semantics TODO: `Unsupported: parameter type: union U` [SemanticsTodo]
+  // CHECK:STDERR:   Cpp.foo2({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR: fail_todo_import_double_decl_value_param_type.carbon:[[@LINE+4]]:3: note: in `Cpp` name lookup for `foo2` [InCppNameLookup]
+  // CHECK:STDERR:   Cpp.foo2({});
+  // CHECK:STDERR:   ^~~~~~~~
+  // CHECK:STDERR:
+  Cpp.foo2({});
+}
+
 // ============================================================================
 // Defined union without data members as parameter type
 // ============================================================================