When building a module, forward diagnostics to the outer diagnostic consumer.
Previously, we would clone the current diagnostic consumer to produce
a new diagnostic consumer to use when building a module. The problem
here is that we end up losing diagnostics for important diagnostic
consumers, such as serialized diagnostics (where we'd end up with two
diagnostic consumers writing the same output file). With forwarding,
the diagnostics from all of the different modules being built get
forwarded to the one serialized-diagnostic consumer and are emitted in
a sane way.
Fixes <rdar://problem/13663996>.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@181067 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h
index 0327636..2d9d288 100644
--- a/include/clang/Basic/Diagnostic.h
+++ b/include/clang/Basic/Diagnostic.h
@@ -1319,6 +1319,26 @@
}
};
+/// \brief Diagnostic consumer that forwards diagnostics along to an
+/// existing, already-initialized diagnostic consumer.
+///
+class ForwardingDiagnosticConsumer : public DiagnosticConsumer {
+ DiagnosticConsumer &Target;
+
+public:
+ ForwardingDiagnosticConsumer(DiagnosticConsumer &Target) : Target(Target) {}
+
+ virtual ~ForwardingDiagnosticConsumer();
+
+ virtual void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+ const Diagnostic &Info);
+ virtual void clear();
+
+ virtual bool IncludeInDiagnosticCounts() const;
+
+ virtual DiagnosticConsumer *clone(DiagnosticsEngine &Diags) const;
+};
+
// Struct used for sending info about how a type should be printed.
struct TemplateDiffTypes {
intptr_t FromType;
diff --git a/include/clang/Lex/ModuleMap.h b/include/clang/Lex/ModuleMap.h
index 33c92f5..dc75f18 100644
--- a/include/clang/Lex/ModuleMap.h
+++ b/include/clang/Lex/ModuleMap.h
@@ -161,7 +161,7 @@
/// \param LangOpts Language options for this translation unit.
///
/// \param Target The target for this translation unit.
- ModuleMap(FileManager &FileMgr, const DiagnosticConsumer &DC,
+ ModuleMap(FileManager &FileMgr, DiagnosticConsumer &DC,
const LangOptions &LangOpts, const TargetInfo *Target,
HeaderSearch &HeaderInfo);
diff --git a/lib/Basic/Diagnostic.cpp b/lib/Basic/Diagnostic.cpp
index 4a5b191..5eff86c 100644
--- a/lib/Basic/Diagnostic.cpp
+++ b/lib/Basic/Diagnostic.cpp
@@ -972,6 +972,28 @@
void IgnoringDiagConsumer::anchor() { }
+ForwardingDiagnosticConsumer::~ForwardingDiagnosticConsumer() {}
+
+void ForwardingDiagnosticConsumer::HandleDiagnostic(
+ DiagnosticsEngine::Level DiagLevel,
+ const Diagnostic &Info) {
+ Target.HandleDiagnostic(DiagLevel, Info);
+}
+
+void ForwardingDiagnosticConsumer::clear() {
+ DiagnosticConsumer::clear();
+ Target.clear();
+}
+
+bool ForwardingDiagnosticConsumer::IncludeInDiagnosticCounts() const {
+ return Target.IncludeInDiagnosticCounts();
+}
+
+DiagnosticConsumer *
+ForwardingDiagnosticConsumer::clone(DiagnosticsEngine &Diags) const {
+ return new ForwardingDiagnosticConsumer(Target);
+}
+
PartialDiagnostic::StorageAllocator::StorageAllocator() {
for (unsigned I = 0; I != NumCached; ++I)
FreeList[I] = Cached + I;
diff --git a/lib/Frontend/ASTMerge.cpp b/lib/Frontend/ASTMerge.cpp
index bfb3083..b6c644e 100644
--- a/lib/Frontend/ASTMerge.cpp
+++ b/lib/Frontend/ASTMerge.cpp
@@ -34,7 +34,7 @@
void ASTMergeAction::ExecuteAction() {
CompilerInstance &CI = getCompilerInstance();
CI.getDiagnostics().getClient()->BeginSourceFile(
- CI.getASTContext().getLangOpts());
+ CI.getASTContext().getLangOpts());
CI.getDiagnostics().SetArgToStringFn(&FormatASTNodeDiagnosticArgument,
&CI.getASTContext());
IntrusiveRefCntPtr<DiagnosticIDs>
@@ -42,8 +42,9 @@
for (unsigned I = 0, N = ASTFiles.size(); I != N; ++I) {
IntrusiveRefCntPtr<DiagnosticsEngine>
Diags(new DiagnosticsEngine(DiagIDs, &CI.getDiagnosticOpts(),
- CI.getDiagnostics().getClient(),
- /*ShouldOwnClient=*/false));
+ new ForwardingDiagnosticConsumer(
+ *CI.getDiagnostics().getClient()),
+ /*ShouldOwnClient=*/true));
ASTUnit *Unit = ASTUnit::LoadFromASTFile(ASTFiles[I], Diags,
CI.getFileSystemOpts(), false);
if (!Unit)
diff --git a/lib/Frontend/ASTUnit.cpp b/lib/Frontend/ASTUnit.cpp
index c1115ae..1b4e5a0 100644
--- a/lib/Frontend/ASTUnit.cpp
+++ b/lib/Frontend/ASTUnit.cpp
@@ -236,6 +236,11 @@
}
ASTUnit::~ASTUnit() {
+ // If we loaded from an AST file, balance out the BeginSourceFile call.
+ if (MainFileIsAST && getDiagnostics().getClient()) {
+ getDiagnostics().getClient()->EndSourceFile();
+ }
+
clearFileLevelDecls();
// Clean up the temporary files and the preamble file.
@@ -581,14 +586,22 @@
}
};
+ /// \brief Diagnostic consumer that saves each diagnostic it is given.
class StoredDiagnosticConsumer : public DiagnosticConsumer {
SmallVectorImpl<StoredDiagnostic> &StoredDiags;
-
+ SourceManager *SourceMgr;
+
public:
explicit StoredDiagnosticConsumer(
SmallVectorImpl<StoredDiagnostic> &StoredDiags)
- : StoredDiags(StoredDiags) { }
-
+ : StoredDiags(StoredDiags), SourceMgr(0) { }
+
+ virtual void BeginSourceFile(const LangOptions &LangOpts,
+ const Preprocessor *PP = 0) {
+ if (PP)
+ SourceMgr = &PP->getSourceManager();
+ }
+
virtual void HandleDiagnostic(DiagnosticsEngine::Level Level,
const Diagnostic &Info);
@@ -635,7 +648,11 @@
// Default implementation (Warnings/errors count).
DiagnosticConsumer::HandleDiagnostic(Level, Info);
- StoredDiags.push_back(StoredDiagnostic(Level, Info));
+ // Only record the diagnostic if it's part of the source manager we know
+ // about. This effectively drops diagnostics from modules we're building.
+ // FIXME: In the long run, ee don't want to drop source managers from modules.
+ if (!Info.hasSourceManager() || &Info.getSourceManager() == SourceMgr)
+ StoredDiags.push_back(StoredDiagnostic(Level, Info));
}
ASTDeserializationListener *ASTUnit::getDeserializationListener() {
@@ -835,6 +852,9 @@
ReaderPtr->InitializeSema(*AST->TheSema);
AST->Reader = ReaderPtr;
+ // Tell the diagnostic client that we have started a source file.
+ AST->getDiagnostics().getClient()->BeginSourceFile(Context.getLangOpts(),&PP);
+
return AST.take();
}
diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp
index df06a81..027e99a 100644
--- a/lib/Frontend/CompilerInstance.cpp
+++ b/lib/Frontend/CompilerInstance.cpp
@@ -868,9 +868,11 @@
// module.
CompilerInstance Instance;
Instance.setInvocation(&*Invocation);
- Instance.createDiagnostics(&ImportingInstance.getDiagnosticClient(),
+
+ Instance.createDiagnostics(new ForwardingDiagnosticConsumer(
+ ImportingInstance.getDiagnosticClient()),
/*ShouldOwnClient=*/true,
- /*ShouldCloneClient=*/true);
+ /*ShouldCloneClient=*/false);
// Note that this module is part of the module build stack, so that we
// can detect cycles in the module graph.
@@ -892,6 +894,7 @@
llvm::CrashRecoveryContext CRC;
CompileModuleMapData Data = { Instance, CreateModuleAction };
CRC.RunSafelyOnThread(&doCompileMapModule, &Data, ThreadStackSize);
+
// Delete the temporary module map file.
// FIXME: Even though we're executing under crash protection, it would still
diff --git a/lib/Frontend/VerifyDiagnosticConsumer.cpp b/lib/Frontend/VerifyDiagnosticConsumer.cpp
index 4f05aef..91aa52b 100644
--- a/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ b/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -111,8 +111,13 @@
void VerifyDiagnosticConsumer::HandleDiagnostic(
DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
- if (Info.hasSourceManager())
+ if (Info.hasSourceManager()) {
+ // If this diagnostic is for a different source manager, ignore it.
+ if (SrcManager && &Info.getSourceManager() != SrcManager)
+ return;
+
setSourceManager(Info.getSourceManager());
+ }
#ifndef NDEBUG
// Debug build tracks unparsed files for possible
@@ -476,6 +481,11 @@
bool VerifyDiagnosticConsumer::HandleComment(Preprocessor &PP,
SourceRange Comment) {
SourceManager &SM = PP.getSourceManager();
+
+ // If this comment is for a different source manager, ignore it.
+ if (SrcManager && &SM != SrcManager)
+ return false;
+
SourceLocation CommentBegin = Comment.getBegin();
const char *CommentRaw = SM.getCharacterData(CommentBegin);
diff --git a/lib/Lex/ModuleMap.cpp b/lib/Lex/ModuleMap.cpp
index 1458a73..3e7a44c 100644
--- a/lib/Lex/ModuleMap.cpp
+++ b/lib/Lex/ModuleMap.cpp
@@ -83,7 +83,7 @@
return Context;
}
-ModuleMap::ModuleMap(FileManager &FileMgr, const DiagnosticConsumer &DC,
+ModuleMap::ModuleMap(FileManager &FileMgr, DiagnosticConsumer &DC,
const LangOptions &LangOpts, const TargetInfo *Target,
HeaderSearch &HeaderInfo)
: LangOpts(LangOpts), Target(Target), HeaderInfo(HeaderInfo),
@@ -92,7 +92,8 @@
IntrusiveRefCntPtr<DiagnosticIDs> DiagIDs(new DiagnosticIDs);
Diags = IntrusiveRefCntPtr<DiagnosticsEngine>(
new DiagnosticsEngine(DiagIDs, new DiagnosticOptions));
- Diags->setClient(DC.clone(*Diags), /*ShouldOwnClient=*/true);
+ Diags->setClient(new ForwardingDiagnosticConsumer(DC),
+ /*ShouldOwnClient=*/true);
SourceMgr = new SourceManager(*Diags, FileMgr);
}
diff --git a/test/Modules/Inputs/ModuleDiags/has_errors.h b/test/Modules/Inputs/ModuleDiags/has_errors.h
new file mode 100644
index 0000000..2c0929a
--- /dev/null
+++ b/test/Modules/Inputs/ModuleDiags/has_errors.h
@@ -0,0 +1,2 @@
+static void foo(void) { }
+static void foo(void) { }
diff --git a/test/Modules/Inputs/ModuleDiags/has_warnings.h b/test/Modules/Inputs/ModuleDiags/has_warnings.h
new file mode 100644
index 0000000..87112be
--- /dev/null
+++ b/test/Modules/Inputs/ModuleDiags/has_warnings.h
@@ -0,0 +1,3 @@
+
+int int_val;
+float *float_ptr = &int_val;
diff --git a/test/Modules/Inputs/ModuleDiags/module.map b/test/Modules/Inputs/ModuleDiags/module.map
new file mode 100644
index 0000000..09b2508
--- /dev/null
+++ b/test/Modules/Inputs/ModuleDiags/module.map
@@ -0,0 +1,7 @@
+module HasWarnings {
+ header "has_warnings.h"
+}
+
+module HasErrors {
+ header "has_errors.h"
+}
diff --git a/test/Modules/cycles.c b/test/Modules/cycles.c
index 4326e76..5f83092 100644
--- a/test/Modules/cycles.c
+++ b/test/Modules/cycles.c
@@ -6,8 +6,8 @@
// CHECK: While building module 'MutuallyRecursive1' imported from
// CHECK: While building module 'MutuallyRecursive2' imported from
// CHECK: MutuallyRecursive2.h:3:9: fatal error: cyclic dependency in module 'MutuallyRecursive1': MutuallyRecursive1 -> MutuallyRecursive2 -> MutuallyRecursive1
-// CHECK: While building module 'MutuallyRecursive1' imported from
// CHECK: MutuallyRecursive1.h:2:9: fatal error: could not build module 'MutuallyRecursive2'
// CHECK: cycles.c:4:9: fatal error: could not build module 'MutuallyRecursive1'
-// CHECK-NOT: error:
+// CHECK: 3 errors generated
+
diff --git a/test/Modules/serialized-diags.m b/test/Modules/serialized-diags.m
new file mode 100644
index 0000000..99ba80a
--- /dev/null
+++ b/test/Modules/serialized-diags.m
@@ -0,0 +1,32 @@
+@import HasWarnings;
+
+#ifdef WITH_ERRORS
+@import HasErrors;
+#endif
+
+float float_val;
+double *double_ptr = &float_val;
+
+// RUN: rm -rf %t %t.diag %t.out
+// RUN: %clang -fmodules -fmodules-cache-path=%t/ModuleCache -I %S/Inputs/ModuleDiags -fsyntax-only %s --serialize-diagnostics %t.diag > /dev/null 2>&1
+// RUN: c-index-test -read-diagnostics %t.diag > %t.out 2>&1
+// RUN: FileCheck --input-file=%t.out %s
+
+// CHECK: Inputs/ModuleDiags/has_warnings.h:3:8: warning: incompatible pointer types initializing 'float *'
+// CHECK: serialized-diags.m:1:9: note: while building module 'HasWarnings' imported from
+// CHECK: serialized-diags.m:8:9: warning: incompatible pointer types initializing 'double *'
+// CHECK: Number of diagnostics: 2
+
+// RUN: rm -rf %t %t.diag_errors %t.out_errors
+// RUN: not %clang -fmodules -fmodules-cache-path=%t/ModuleCache -I %S/Inputs/ModuleDiags -fsyntax-only -DWITH_ERRORS %s --serialize-diagnostics %t.diag_errors > /dev/null 2>&1
+// RUN: c-index-test -read-diagnostics %t.diag_errors > %t.out_errors 2>&1
+// RUN: FileCheck -check-prefix=CHECK-WITH-ERRORS --input-file=%t.out_errors %s
+
+// CHECK-WITH-ERRORS: has_warnings.h:3:8: warning: incompatible pointer types initializing 'float *'
+// CHECK-WITH-ERRORS: serialized-diags.m:1:9: note: while building module 'HasWarnings'
+// CHECK-WITH-ERRORS: has_errors.h:2:13: error: redefinition of 'foo'
+// CHECK-WITH-ERRORS: serialized-diags.m:4:9: note: while building module 'HasErrors'
+// CHECK-WITH-ERRORS: has_errors.h:1:13: note: previous definition is here
+// CHECK-WITH-ERRORS: serialized-diags.m:4:9: fatal: could not build module 'HasErrors'
+// CHECK-WITH-ERRORS: Number of diagnostics: 3
+