From 805b3eebce92ec64cde92d49f7e58a6c58978068 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 21 May 2026 14:21:50 -0700 Subject: [PATCH] Fix crash if initializing Clang fails. (#7248) Flush diagnostics before destroying Clang. If we see an `inline Cpp` and `Cpp` initialization failed, recover by skipping the inline code rather than CHECK-failing. --- toolchain/check/cpp/generate_ast.cpp | 11 +++++---- toolchain/check/handle_inline_decl.cpp | 18 +++++++++++---- .../interop/cpp/basics/import/bad_args.carbon | 23 +++++++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 toolchain/check/testdata/interop/cpp/basics/import/bad_args.carbon diff --git a/toolchain/check/cpp/generate_ast.cpp b/toolchain/check/cpp/generate_ast.cpp index a42e8c8a0351..13c8ab19e0ca 100644 --- a/toolchain/check/cpp/generate_ast.cpp +++ b/toolchain/check/cpp/generate_ast.cpp @@ -882,6 +882,13 @@ auto GenerateAst(Context& context, context.sem_ir().set_cpp_file(std::make_unique( std::move(clang_instance_ptr), llvm_context)); + // Register an annotation scope to flush any Clang diagnostics when we return. + // This ensures C++ diagnostics get flushed before `diags` is destroyed, and + // that diagnostics created here don't interleave with later Carbon + // diagnostics. + Diagnostics::AnnotationScope annotate_diagnostics(&context.emitter(), + [](auto& /*builder*/) {}); + clang_instance.setDiagnostics(diags); clang_instance.setVirtualFileSystem(fs); clang_instance.createFileManager(); @@ -915,10 +922,6 @@ auto GenerateAst(Context& context, return false; } - // Flush any diagnostics. We know we're not part-way through emitting a - // diagnostic now. - context.emitter().Flush(); - return true; } diff --git a/toolchain/check/handle_inline_decl.cpp b/toolchain/check/handle_inline_decl.cpp index e70530f6f22e..c119add2d7aa 100644 --- a/toolchain/check/handle_inline_decl.cpp +++ b/toolchain/check/handle_inline_decl.cpp @@ -30,12 +30,22 @@ auto HandleParseNode(Context& context, Parse::InlineCppDeclId node_id) -> bool { // TODO: It'd be nice to produce a clearer error saying to insert an `import // Cpp` in a file that uses `inline Cpp` and doesn't otherwise import anything // from package `Cpp`. - if (context.constant_values().Get( - context.node_stack().Pop()) == - SemIR::ErrorInst::ConstantId) { + auto cpp_id = context.constant_values().Get( + context.node_stack().Pop()); + if (cpp_id == SemIR::ErrorInst::ConstantId) { + return true; + } + + // If Clang initialization catastrophically failed, skip the inline fragment. + if (!context.cpp_context()) { + // We should have already diagnosed an error initializing Clang. + auto cpp_scope_id = context.constant_values() + .GetInstAs(cpp_id) + .name_scope_id; + CARBON_CHECK(context.name_scopes().Get(cpp_scope_id).has_error(), + "Have valid `Cpp` scope but no Cpp context"); return true; } - CARBON_CHECK(context.cpp_context(), "Have `Cpp` name but no Cpp context"); auto string_token = context.parse_tree().node_token(body_id); auto string_value_id = context.tokens().GetStringLiteralValue(string_token); diff --git a/toolchain/check/testdata/interop/cpp/basics/import/bad_args.carbon b/toolchain/check/testdata/interop/cpp/basics/import/bad_args.carbon new file mode 100644 index 000000000000..b4d3a8b0b269 --- /dev/null +++ b/toolchain/check/testdata/interop/cpp/basics/import/bad_args.carbon @@ -0,0 +1,23 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/none.carbon +// EXTRA-ARGS: --clang-arg=-fmodules --clang-arg=-fmodule-file=nonexistent.pcm +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/interop/cpp/basics/import/bad_args.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interop/cpp/basics/import/bad_args.carbon +// CHECK:STDERR: error: module file 'nonexistent.pcm' not found [CppInteropParseError] +// CHECK:STDERR: + +// --- fail_use_cpp.carbon + +library "[[@TEST_NAME]]"; + +import Cpp; +inline Cpp ''' +int n; +''';