Skip to content

Commit

Permalink
Update VerifyDiagnosticConsumer to only get directives during parsing.
Browse files Browse the repository at this point in the history
The old behavior was to re-scan any files (like modules) where we may have
directives but won't actually be parsing during the -verify invocation.
Now, we keep the old behavior in Debug builds as a sanity check (though
modules are a known entity), and expect all legitimate directives to come
from comments seen by the preprocessor.

This also affects the ARC migration tool, which captures diagnostics in
order to filter some out. This change adds an explicit cleanup to
CaptureDiagnosticsConsumer in order to let its sub-consumer handle the
real end of diagnostics.

This was originally split into four patches, but the tests do not run
cleanly without all four, so I've combined them into one commit.

Patches by Andy Gibbs, with slight modifications from me.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161650 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
jrose-apple committed Aug 10, 2012
1 parent af6cf43 commit 7c304f5
Show file tree
Hide file tree
Showing 11 changed files with 220 additions and 50 deletions.
18 changes: 15 additions & 3 deletions include/clang/Frontend/VerifyDiagnosticConsumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,17 +166,22 @@ class VerifyDiagnosticConsumer: public DiagnosticConsumer,
}
};

private:
#ifndef NDEBUG
typedef llvm::DenseSet<FileID> FilesWithDiagnosticsSet;
typedef llvm::SmallPtrSet<const FileEntry *, 4> FilesWithDirectivesSet;
typedef llvm::SmallPtrSet<const FileEntry *, 4> FilesParsedForDirectivesSet;
#endif

private:
DiagnosticsEngine &Diags;
DiagnosticConsumer *PrimaryClient;
bool OwnsPrimaryClient;
OwningPtr<TextDiagnosticBuffer> Buffer;
const Preprocessor *CurrentPreprocessor;
unsigned ActiveSourceFiles;
#ifndef NDEBUG
FilesWithDiagnosticsSet FilesWithDiagnostics;
FilesWithDirectivesSet FilesWithDirectives;
FilesParsedForDirectivesSet FilesParsedForDirectives;
#endif
ExpectedData ED;
void CheckDiagnostics();

Expand All @@ -192,6 +197,13 @@ class VerifyDiagnosticConsumer: public DiagnosticConsumer,

virtual void EndSourceFile();

/// \brief Manually register a file as parsed.
inline void appendParsedFile(const FileEntry *File) {
#ifndef NDEBUG
FilesParsedForDirectives.insert(File);
#endif
}

virtual bool HandleComment(Preprocessor &PP, SourceRange Comment);

virtual void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
Expand Down
49 changes: 43 additions & 6 deletions lib/ARCMigrate/ARCMT.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,40 @@ namespace {

class CaptureDiagnosticConsumer : public DiagnosticConsumer {
DiagnosticsEngine &Diags;
DiagnosticConsumer &DiagClient;
CapturedDiagList &CapturedDiags;
bool HasBegunSourceFile;
public:
CaptureDiagnosticConsumer(DiagnosticsEngine &diags,
CapturedDiagList &capturedDiags)
: Diags(diags), CapturedDiags(capturedDiags) { }
DiagnosticConsumer &client,
CapturedDiagList &capturedDiags)
: Diags(diags), DiagClient(client), CapturedDiags(capturedDiags),
HasBegunSourceFile(false) { }

virtual void BeginSourceFile(const LangOptions &Opts,
const Preprocessor *PP) {
// Pass BeginSourceFile message onto DiagClient on first call.
// The corresponding EndSourceFile call will be made from an
// explicit call to FinishCapture.
if (!HasBegunSourceFile) {
DiagClient.BeginSourceFile(Opts, PP);
HasBegunSourceFile = true;
}
}

void FinishCapture() {
// Call EndSourceFile on DiagClient on completion of capture to
// enable VerifyDiagnosticConsumer to check diagnostics *after*
// it has received the diagnostic list.
if (HasBegunSourceFile) {
DiagClient.EndSourceFile();
HasBegunSourceFile = false;
}
}

virtual ~CaptureDiagnosticConsumer() {
assert(!HasBegunSourceFile && "FinishCapture not called!");
}

virtual void HandleDiagnostic(DiagnosticsEngine::Level level,
const Diagnostic &Info) {
Expand Down Expand Up @@ -260,13 +289,15 @@ bool arcmt::checkForManualIssues(CompilerInvocation &origCI,
new DiagnosticsEngine(DiagID, DiagClient, /*ShouldOwnClient=*/false));

// Filter of all diagnostics.
CaptureDiagnosticConsumer errRec(*Diags, capturedDiags);
CaptureDiagnosticConsumer errRec(*Diags, *DiagClient, capturedDiags);
Diags->setClient(&errRec, /*ShouldOwnClient=*/false);

OwningPtr<ASTUnit> Unit(
ASTUnit::LoadFromCompilerInvocationAction(CInvok.take(), Diags));
if (!Unit)
if (!Unit) {
errRec.FinishCapture();
return true;
}

// Don't filter diagnostics anymore.
Diags->setClient(DiagClient, /*ShouldOwnClient=*/false);
Expand All @@ -278,6 +309,7 @@ bool arcmt::checkForManualIssues(CompilerInvocation &origCI,
DiagClient->BeginSourceFile(Ctx.getLangOpts(), &Unit->getPreprocessor());
capturedDiags.reportDiagnostics(*Diags);
DiagClient->EndSourceFile();
errRec.FinishCapture();
return true;
}

Expand Down Expand Up @@ -315,6 +347,7 @@ bool arcmt::checkForManualIssues(CompilerInvocation &origCI,
capturedDiags.reportDiagnostics(*Diags);

DiagClient->EndSourceFile();
errRec.FinishCapture();

// If we are migrating code that gets the '-fobjc-arc' flag, make sure
// to remove it so that we don't get errors from normal compilation.
Expand Down Expand Up @@ -563,7 +596,7 @@ bool MigrationProcess::applyTransform(TransformFn trans,
new DiagnosticsEngine(DiagID, DiagClient, /*ShouldOwnClient=*/false));

// Filter of all diagnostics.
CaptureDiagnosticConsumer errRec(*Diags, capturedDiags);
CaptureDiagnosticConsumer errRec(*Diags, *DiagClient, capturedDiags);
Diags->setClient(&errRec, /*ShouldOwnClient=*/false);

OwningPtr<ARCMTMacroTrackerAction> ASTAction;
Expand All @@ -572,8 +605,10 @@ bool MigrationProcess::applyTransform(TransformFn trans,
OwningPtr<ASTUnit> Unit(
ASTUnit::LoadFromCompilerInvocationAction(CInvok.take(), Diags,
ASTAction.get()));
if (!Unit)
if (!Unit) {
errRec.FinishCapture();
return true;
}
Unit->setOwnsRemappedFileBuffers(false); // FileRemapper manages that.

// Don't filter diagnostics anymore.
Expand All @@ -586,6 +621,7 @@ bool MigrationProcess::applyTransform(TransformFn trans,
DiagClient->BeginSourceFile(Ctx.getLangOpts(), &Unit->getPreprocessor());
capturedDiags.reportDiagnostics(*Diags);
DiagClient->EndSourceFile();
errRec.FinishCapture();
return true;
}

Expand All @@ -609,6 +645,7 @@ bool MigrationProcess::applyTransform(TransformFn trans,
}

DiagClient->EndSourceFile();
errRec.FinishCapture();

if (DiagClient->getNumErrors())
return true;
Expand Down
Loading

0 comments on commit 7c304f5

Please sign in to comment.