r181067 - When building a module, forward diagnostics to the outer diagnostic consumer.
Douglas Gregor
dgregor at apple.com
Fri May 3 15:58:43 PDT 2013
Author: dgregor
Date: Fri May 3 17:58:43 2013
New Revision: 181067
URL: http://llvm.org/viewvc/llvm-project?rev=181067&view=rev
Log:
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>.
Added:
cfe/trunk/test/Modules/Inputs/ModuleDiags/
cfe/trunk/test/Modules/Inputs/ModuleDiags/has_errors.h
cfe/trunk/test/Modules/Inputs/ModuleDiags/has_warnings.h
cfe/trunk/test/Modules/Inputs/ModuleDiags/module.map
cfe/trunk/test/Modules/serialized-diags.m
Modified:
cfe/trunk/include/clang/Basic/Diagnostic.h
cfe/trunk/include/clang/Lex/ModuleMap.h
cfe/trunk/lib/Basic/Diagnostic.cpp
cfe/trunk/lib/Frontend/ASTMerge.cpp
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/test/Modules/cycles.c
Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=181067&r1=181066&r2=181067&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Fri May 3 17:58:43 2013
@@ -1319,6 +1319,26 @@ class IgnoringDiagConsumer : public Diag
}
};
+/// \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;
Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=181067&r1=181066&r2=181067&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Fri May 3 17:58:43 2013
@@ -161,7 +161,7 @@ public:
/// \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);
Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=181067&r1=181066&r2=181067&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
+++ cfe/trunk/lib/Basic/Diagnostic.cpp Fri May 3 17:58:43 2013
@@ -972,6 +972,28 @@ bool DiagnosticConsumer::IncludeInDiagno
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;
Modified: cfe/trunk/lib/Frontend/ASTMerge.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTMerge.cpp?rev=181067&r1=181066&r2=181067&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ASTMerge.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTMerge.cpp Fri May 3 17:58:43 2013
@@ -34,7 +34,7 @@ bool ASTMergeAction::BeginSourceFileActi
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 @@ void ASTMergeAction::ExecuteAction() {
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)
Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=181067&r1=181066&r2=181067&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Fri May 3 17:58:43 2013
@@ -236,6 +236,11 @@ ASTUnit::ASTUnit(bool _MainFileIsAST)
}
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 @@ private:
}
};
+ /// \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 @@ void StoredDiagnosticConsumer::HandleDia
// 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 @@ ASTUnit *ASTUnit::LoadFromASTFile(const
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();
}
Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=181067&r1=181066&r2=181067&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri May 3 17:58:43 2013
@@ -868,9 +868,11 @@ static void compileModule(CompilerInstan
// 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 @@ static void compileModule(CompilerInstan
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
Modified: cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp?rev=181067&r1=181066&r2=181067&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp (original)
+++ cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp Fri May 3 17:58:43 2013
@@ -111,8 +111,13 @@ void VerifyDiagnosticConsumer::EndSource
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 @@ static bool ParseDirective(StringRef S,
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);
Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=181067&r1=181066&r2=181067&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri May 3 17:58:43 2013
@@ -83,7 +83,7 @@ Module *ModuleMap::resolveModuleId(const
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 @@ ModuleMap::ModuleMap(FileManager &FileMg
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);
}
Added: cfe/trunk/test/Modules/Inputs/ModuleDiags/has_errors.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/ModuleDiags/has_errors.h?rev=181067&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/ModuleDiags/has_errors.h (added)
+++ cfe/trunk/test/Modules/Inputs/ModuleDiags/has_errors.h Fri May 3 17:58:43 2013
@@ -0,0 +1,2 @@
+static void foo(void) { }
+static void foo(void) { }
Added: cfe/trunk/test/Modules/Inputs/ModuleDiags/has_warnings.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/ModuleDiags/has_warnings.h?rev=181067&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/ModuleDiags/has_warnings.h (added)
+++ cfe/trunk/test/Modules/Inputs/ModuleDiags/has_warnings.h Fri May 3 17:58:43 2013
@@ -0,0 +1,3 @@
+
+int int_val;
+float *float_ptr = &int_val;
Added: cfe/trunk/test/Modules/Inputs/ModuleDiags/module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/ModuleDiags/module.map?rev=181067&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/ModuleDiags/module.map (added)
+++ cfe/trunk/test/Modules/Inputs/ModuleDiags/module.map Fri May 3 17:58:43 2013
@@ -0,0 +1,7 @@
+module HasWarnings {
+ header "has_warnings.h"
+}
+
+module HasErrors {
+ header "has_errors.h"
+}
Modified: cfe/trunk/test/Modules/cycles.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cycles.c?rev=181067&r1=181066&r2=181067&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cycles.c (original)
+++ cfe/trunk/test/Modules/cycles.c Fri May 3 17:58:43 2013
@@ -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
+
Added: cfe/trunk/test/Modules/serialized-diags.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/serialized-diags.m?rev=181067&view=auto
==============================================================================
--- cfe/trunk/test/Modules/serialized-diags.m (added)
+++ cfe/trunk/test/Modules/serialized-diags.m Fri May 3 17:58:43 2013
@@ -0,0 +1,32 @@
+ at import HasWarnings;
+
+#ifdef WITH_ERRORS
+ at 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
+
More information about the cfe-commits
mailing list