carbon-language/carbon-lang
1
0
Fork 0
mirror of https://github.com/carbon-language/carbon-lang.git synced 2026-10-04 22:02:52 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

Don't use Check::Context when emitting C++ diagnostics. (#5910)

Browse Source 
Diagnostics can get flushed after the context is destroyed. Should fix
an issue found by msan.
This commit is contained in:
Richard Smith
2025-08-05 21:21:30 +00:00
committed by GitHub
parent fd4dbc5b6f
commit bd2483b553
1 changed files with 67 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
toolchain/check/import_cpp.cpp
+67 -57
View File
@@ -93,13 +93,12 @@ static auto AddIdentifierName(Context& context, llvm::StringRef name)
// Adds the given source location and an `ImportIRInst` referring to it in
// `ImportIRId::Cpp`.
static auto AddImportIRInst(Context& context,
static auto AddImportIRInst(SemIR::File& file,
clang::SourceLocation clang_source_loc)
-> SemIR::ImportIRInstId {
SemIR::ClangSourceLocId clang_source_loc_id =
context.sem_ir().clang_source_locs().Add(clang_source_loc);
return context.import_ir_insts().Add(
SemIR::ImportIRInst(clang_source_loc_id));
file.clang_source_locs().Add(clang_source_loc);
return file.import_ir_insts().Add(SemIR::ImportIRInst(clang_source_loc_id));
}
namespace {
@@ -116,15 +115,22 @@ namespace {
// wrong C++ diagnostics, and at the end of the Carbon program.
class CarbonClangDiagnosticConsumer : public clang::DiagnosticConsumer {
public:
// Creates an instance with the location that triggers calling Clang.
// `context` must not be null.
// Creates an instance with the location that triggers calling Clang. The
// `context` is not stored here, and the diagnostics consumer is expected to
// outlive it.
explicit CarbonClangDiagnosticConsumer(
Context* context, std::shared_ptr<clang::CompilerInvocation> invocation)
: context_(context), invocation_(std::move(invocation)) {
context->emitter().AddFlushFn([this] { EmitDiagnostics(); });
Context& context, std::shared_ptr<clang::CompilerInvocation> invocation)
: sem_ir_(&context.sem_ir()),
emitter_(&context.emitter()),
invocation_(std::move(invocation)) {
emitter_->AddFlushFn([this] { EmitDiagnostics(); });
}
~CarbonClangDiagnosticConsumer() override {
// Do not inspect `emitter_` here; it's typically destroyed before the
// consumer is.
// TODO: If Clang produces diagnostics after check finishes, they'll get
// added to the list of pending diagnostics and never emitted.
CARBON_CHECK(diagnostic_infos_.empty(),
"Missing flush before destroying diagnostic consumer");
}
@@ -136,7 +142,7 @@ class CarbonClangDiagnosticConsumer : public clang::DiagnosticConsumer {
DiagnosticConsumer::HandleDiagnostic(diag_level, info);
SemIR::ImportIRInstId clang_import_ir_inst_id =
AddImportIRInst(*context_, info.getLocation());
AddImportIRInst(*sem_ir_, info.getLocation());
llvm::SmallString<256> message;
info.FormatDiagnostic(message);
@@ -163,57 +169,57 @@ class CarbonClangDiagnosticConsumer : public clang::DiagnosticConsumer {
.snippet = snippet_stream.TakeStr()});
}
// Returns the diagnostic to use for a given Clang diagnostic level.
static auto GetDiagnostic(clang::DiagnosticsEngine::Level level)
-> const Diagnostics::DiagnosticBase<std::string>& {
switch (level) {
case clang::DiagnosticsEngine::Ignored: {
CARBON_FATAL("Emitting an ignored diagnostic");
break;
}
case clang::DiagnosticsEngine::Note: {
CARBON_DIAGNOSTIC(CppInteropParseNote, Note, "{0}", std::string);
return CppInteropParseNote;
}
case clang::DiagnosticsEngine::Remark:
case clang::DiagnosticsEngine::Warning: {
// TODO: Add a distinct Remark level to Carbon diagnostics, and stop
// mapping remarks to warnings.
CARBON_DIAGNOSTIC(CppInteropParseWarning, Warning, "{0}", std::string);
return CppInteropParseWarning;
}
case clang::DiagnosticsEngine::Error:
case clang::DiagnosticsEngine::Fatal: {
CARBON_DIAGNOSTIC(CppInteropParseError, Error, "{0}", std::string);
return CppInteropParseError;
}
}
}
// Outputs Carbon diagnostics based on the collected Clang diagnostics. Must
// be called after the AST is set in the context.
auto EmitDiagnostics() -> void {
CARBON_CHECK(context_->sem_ir().cpp_ast(),
CARBON_CHECK(sem_ir_->cpp_ast(),
"Attempted to emit diagnostics before the AST Unit is loaded");
for (size_t i = 0; i != diagnostic_infos_.size(); ++i) {
const ClangDiagnosticInfo& info = diagnostic_infos_[i];
switch (info.level) {
case clang::DiagnosticsEngine::Ignored:
case clang::DiagnosticsEngine::Note:
case clang::DiagnosticsEngine::Remark: {
context_->TODO(
SemIR::LocId(info.import_ir_inst_id),
llvm::formatv(
"Unsupported: C++ diagnostic level for diagnostic\n{0}",
info.message));
break;
}
case clang::DiagnosticsEngine::Warning:
case clang::DiagnosticsEngine::Error:
case clang::DiagnosticsEngine::Fatal: {
CARBON_DIAGNOSTIC(CppInteropParseWarning, Warning, "{0}",
std::string);
CARBON_DIAGNOSTIC(CppInteropParseError, Error, "{0}", std::string);
auto builder = context_->emitter().Build(
SemIR::LocId(info.import_ir_inst_id),
info.level == clang::DiagnosticsEngine::Warning
? CppInteropParseWarning
: CppInteropParseError,
info.message);
builder.OverrideSnippet(info.snippet);
for (;
i + 1 < diagnostic_infos_.size() &&
diagnostic_infos_[i + 1].level == clang::DiagnosticsEngine::Note;
++i) {
const ClangDiagnosticInfo& note_info = diagnostic_infos_[i + 1];
CARBON_DIAGNOSTIC(CppInteropParseNote, Note, "{0}", std::string);
builder
.Note(SemIR::LocId(note_info.import_ir_inst_id),
CppInteropParseNote, note_info.message)
.OverrideSnippet(note_info.snippet);
}
// TODO: This will apply all current Carbon annotation functions. We
// should instead track how Clang's context notes and Carbon's
// annotation functions are interleaved, and interleave the notes in
// the same order.
builder.Emit();
break;
}
auto builder = emitter_->Build(SemIR::LocId(info.import_ir_inst_id),
GetDiagnostic(info.level), info.message);
builder.OverrideSnippet(info.snippet);
for (; i + 1 < diagnostic_infos_.size() &&
diagnostic_infos_[i + 1].level == clang::DiagnosticsEngine::Note;
++i) {
const ClangDiagnosticInfo& note_info = diagnostic_infos_[i + 1];
builder
.Note(SemIR::LocId(note_info.import_ir_inst_id),
GetDiagnostic(note_info.level), note_info.message)
.OverrideSnippet(note_info.snippet);
}
// TODO: This will apply all current Carbon annotation functions. We
// should instead track how Clang's context notes and Carbon's annotation
// functions are interleaved, and interleave the notes in the same order.
builder.Emit();
}
diagnostic_infos_.clear();
}
@@ -267,8 +273,11 @@ class CarbonClangDiagnosticConsumer : public clang::DiagnosticConsumer {
std::string snippet;
};
// The type-checking context in which we're running Clang.
Context* context_;
// The Carbon file that this C++ compilation is attached to.
SemIR::File* sem_ir_;
// The diagnostic emitter that we're emitting diagnostics into.
DiagnosticEmitterBase* emitter_;
// The compiler invocation that is producing the diagnostics.
std::shared_ptr<clang::CompilerInvocation> invocation_;
@@ -316,7 +325,7 @@ static auto GenerateAst(
llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine> diags(
clang::CompilerInstance::createDiagnostics(
*fs, invocation->getDiagnosticOpts(),
new CarbonClangDiagnosticConsumer(&context, invocation),
new CarbonClangDiagnosticConsumer(context, invocation),
/*ShouldOwnClient=*/true));
// Extract the input from the frontend invocation and make sure it makes
@@ -587,7 +596,8 @@ static auto BuildClassDecl(Context& context,
static auto ImportCXXRecordDecl(Context& context,
clang::CXXRecordDecl* clang_decl)
-> SemIR::InstId {
auto import_ir_inst_id = AddImportIRInst(context, clang_decl->getLocation());
auto import_ir_inst_id =
AddImportIRInst(context.sem_ir(), clang_decl->getLocation());
auto [class_id, class_inst_id] = BuildClassDecl(
context, import_ir_inst_id, GetParentNameScopeId(context, clang_decl),