diff --git a/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h b/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h index 4a6081b0695d..a74589edc94a 100644 --- a/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h +++ b/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h @@ -166,17 +166,22 @@ public: } }; -private: +#ifndef NDEBUG typedef llvm::DenseSet FilesWithDiagnosticsSet; - typedef llvm::SmallPtrSet FilesWithDirectivesSet; + typedef llvm::SmallPtrSet FilesParsedForDirectivesSet; +#endif +private: DiagnosticsEngine &Diags; DiagnosticConsumer *PrimaryClient; bool OwnsPrimaryClient; OwningPtr Buffer; const Preprocessor *CurrentPreprocessor; + unsigned ActiveSourceFiles; +#ifndef NDEBUG FilesWithDiagnosticsSet FilesWithDiagnostics; - FilesWithDirectivesSet FilesWithDirectives; + FilesParsedForDirectivesSet FilesParsedForDirectives; +#endif ExpectedData ED; void CheckDiagnostics(); @@ -192,6 +197,13 @@ public: 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, diff --git a/clang/lib/ARCMigrate/ARCMT.cpp b/clang/lib/ARCMigrate/ARCMT.cpp index dd9461b33d1c..f291dec21fda 100644 --- a/clang/lib/ARCMigrate/ARCMT.cpp +++ b/clang/lib/ARCMigrate/ARCMT.cpp @@ -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) { @@ -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 Unit( ASTUnit::LoadFromCompilerInvocationAction(CInvok.take(), Diags)); - if (!Unit) + if (!Unit) { + errRec.FinishCapture(); return true; + } // Don't filter diagnostics anymore. Diags->setClient(DiagClient, /*ShouldOwnClient=*/false); @@ -278,6 +309,7 @@ bool arcmt::checkForManualIssues(CompilerInvocation &origCI, DiagClient->BeginSourceFile(Ctx.getLangOpts(), &Unit->getPreprocessor()); capturedDiags.reportDiagnostics(*Diags); DiagClient->EndSourceFile(); + errRec.FinishCapture(); return true; } @@ -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. @@ -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 ASTAction; @@ -572,8 +605,10 @@ bool MigrationProcess::applyTransform(TransformFn trans, OwningPtr 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. @@ -586,6 +621,7 @@ bool MigrationProcess::applyTransform(TransformFn trans, DiagClient->BeginSourceFile(Ctx.getLangOpts(), &Unit->getPreprocessor()); capturedDiags.reportDiagnostics(*Diags); DiagClient->EndSourceFile(); + errRec.FinishCapture(); return true; } @@ -609,6 +645,7 @@ bool MigrationProcess::applyTransform(TransformFn trans, } DiagClient->EndSourceFile(); + errRec.FinishCapture(); if (DiagClient->getNumErrors()) return true; diff --git a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp index 8aa65f1f1fc5..71c3f6bce56f 100644 --- a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp +++ b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp @@ -15,6 +15,7 @@ #include "clang/Frontend/VerifyDiagnosticConsumer.h" #include "clang/Frontend/FrontendDiagnostic.h" #include "clang/Frontend/TextDiagnosticBuffer.h" +#include "clang/Lex/HeaderSearch.h" #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/Regex.h" @@ -26,46 +27,91 @@ typedef VerifyDiagnosticConsumer::DirectiveList DirectiveList; typedef VerifyDiagnosticConsumer::ExpectedData ExpectedData; VerifyDiagnosticConsumer::VerifyDiagnosticConsumer(DiagnosticsEngine &_Diags) - : Diags(_Diags), PrimaryClient(Diags.getClient()), - OwnsPrimaryClient(Diags.ownsClient()), - Buffer(new TextDiagnosticBuffer()), CurrentPreprocessor(0) + : Diags(_Diags), + PrimaryClient(Diags.getClient()), OwnsPrimaryClient(Diags.ownsClient()), + Buffer(new TextDiagnosticBuffer()), CurrentPreprocessor(0), + ActiveSourceFiles(0) { Diags.takeClient(); } VerifyDiagnosticConsumer::~VerifyDiagnosticConsumer() { + assert(!ActiveSourceFiles && "Incomplete parsing of source files!"); + assert(!CurrentPreprocessor && "CurrentPreprocessor should be invalid!"); CheckDiagnostics(); Diags.takeClient(); if (OwnsPrimaryClient) delete PrimaryClient; } +#ifndef NDEBUG +namespace { +class VerifyFileTracker : public PPCallbacks { + typedef VerifyDiagnosticConsumer::FilesParsedForDirectivesSet ListType; + ListType &FilesList; + SourceManager &SM; + +public: + VerifyFileTracker(ListType &FilesList, SourceManager &SM) + : FilesList(FilesList), SM(SM) { } + + /// \brief Hook into the preprocessor and update the list of parsed + /// files when the preprocessor indicates a new file is entered. + virtual void FileChanged(SourceLocation Loc, FileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID) { + if (const FileEntry *E = SM.getFileEntryForID(SM.getFileID(Loc))) + FilesList.insert(E); + } +}; +} // End anonymous namespace. +#endif + // DiagnosticConsumer interface. void VerifyDiagnosticConsumer::BeginSourceFile(const LangOptions &LangOpts, const Preprocessor *PP) { - CurrentPreprocessor = PP; - if (PP) const_cast(PP)->addCommentHandler(this); + // Attach comment handler on first invocation. + if (++ActiveSourceFiles == 1) { + if (PP) { + CurrentPreprocessor = PP; + const_cast(PP)->addCommentHandler(this); +#ifndef NDEBUG + VerifyFileTracker *V = new VerifyFileTracker(FilesParsedForDirectives, + PP->getSourceManager()); + const_cast(PP)->addPPCallbacks(V); +#endif + } + } + assert((!PP || CurrentPreprocessor == PP) && "Preprocessor changed!"); PrimaryClient->BeginSourceFile(LangOpts, PP); } void VerifyDiagnosticConsumer::EndSourceFile() { - if (CurrentPreprocessor) - const_cast(CurrentPreprocessor)->removeCommentHandler(this); - CheckDiagnostics(); - + assert(ActiveSourceFiles && "No active source files!"); PrimaryClient->EndSourceFile(); - CurrentPreprocessor = 0; + // Detach comment handler once last active source file completed. + if (--ActiveSourceFiles == 0) { + if (CurrentPreprocessor) + const_cast(CurrentPreprocessor)->removeCommentHandler(this); + + // Check diagnostics once last file completed. + CheckDiagnostics(); + CurrentPreprocessor = 0; + } } void VerifyDiagnosticConsumer::HandleDiagnostic( DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) { +#ifndef NDEBUG if (Info.hasSourceManager()) { - const SourceManager &SM = Info.getSourceManager(); - FilesWithDiagnostics.insert(SM.getFileID(Info.getLocation())); + FileID FID = Info.getSourceManager().getFileID(Info.getLocation()); + if (!FID.isInvalid()) + FilesWithDiagnostics.insert(FID); } +#endif // Send the diagnostic to the buffer, we will check it once we reach the end // of the source file (or are destructed). Buffer->HandleDiagnostic(DiagLevel, Info); @@ -192,7 +238,7 @@ private: /// diagnostics. If so, then put them in the appropriate directive list. /// /// Returns true if any valid directives were found. -static bool ParseDirective(StringRef S, ExpectedData &ED, SourceManager &SM, +static bool ParseDirective(StringRef S, ExpectedData *ED, SourceManager &SM, SourceLocation Pos, DiagnosticsEngine &Diags) { // A single comment may contain multiple directives. bool FoundDirective = false; @@ -210,15 +256,20 @@ static bool ParseDirective(StringRef S, ExpectedData &ED, SourceManager &SM, // Next token: { error | warning | note } DirectiveList* DL = NULL; if (PH.Next("error")) - DL = &ED.Errors; + DL = ED ? &ED->Errors : NULL; else if (PH.Next("warning")) - DL = &ED.Warnings; + DL = ED ? &ED->Warnings : NULL; else if (PH.Next("note")) - DL = &ED.Notes; + DL = ED ? &ED->Notes : NULL; else continue; PH.Advance(); + // If a directive has been found but we're not interested + // in storing the directive information, return now. + if (!DL) + return true; + // Default directive kind. bool RegexKind = false; const char* KindStr = "string"; @@ -360,9 +411,7 @@ bool VerifyDiagnosticConsumer::HandleComment(Preprocessor &PP, // Fold any "\" sequences size_t loc = C.find('\\'); if (loc == StringRef::npos) { - if (ParseDirective(C, ED, SM, CommentBegin, PP.getDiagnostics())) - if (const FileEntry *E = SM.getFileEntryForID(SM.getFileID(CommentBegin))) - FilesWithDirectives.insert(E); + ParseDirective(C, &ED, SM, CommentBegin, PP.getDiagnostics()); return false; } @@ -392,19 +441,20 @@ bool VerifyDiagnosticConsumer::HandleComment(Preprocessor &PP, } if (!C2.empty()) - if (ParseDirective(C2, ED, SM, CommentBegin, PP.getDiagnostics())) - if (const FileEntry *E = SM.getFileEntryForID(SM.getFileID(CommentBegin))) - FilesWithDirectives.insert(E); + ParseDirective(C2, &ED, SM, CommentBegin, PP.getDiagnostics()); return false; } -/// FindExpectedDiags - Lex the main source file to find all of the -// expected errors and warnings. -static void FindExpectedDiags(const Preprocessor &PP, ExpectedData &ED, - FileID FID) { +#ifndef NDEBUG +/// \brief Lex the specified source file to determine whether it contains +/// any expected-* directives. As a Lexer is used rather than a full-blown +/// Preprocessor, directives inside skipped #if blocks will still be found. +/// +/// \return true if any directives were found. +static bool findDirectives(const Preprocessor &PP, FileID FID) { // Create a raw lexer to pull all the comments out of FID. if (FID.isInvalid()) - return; + return false; SourceManager& SM = PP.getSourceManager(); // Create a lexer to lex all the tokens of the main file in raw mode. @@ -416,6 +466,7 @@ static void FindExpectedDiags(const Preprocessor &PP, ExpectedData &ED, Token Tok; Tok.setKind(tok::comment); + bool Found = false; while (Tok.isNot(tok::eof)) { RawLex.Lex(Tok); if (!Tok.is(tok::comment)) continue; @@ -424,9 +475,12 @@ static void FindExpectedDiags(const Preprocessor &PP, ExpectedData &ED, if (Comment.empty()) continue; // Find all expected errors/warnings/notes. - ParseDirective(Comment, ED, SM, Tok.getLocation(), PP.getDiagnostics()); - }; + Found |= ParseDirective(Comment, 0, SM, Tok.getLocation(), + PP.getDiagnostics()); + } + return Found; } +#endif // !NDEBUG /// \brief Takes a list of diagnostics that have been generated but not matched /// by an expected-* directive and produces a diagnostic to the user from this. @@ -556,22 +610,28 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() { // markers. If not then any diagnostics are unexpected. if (CurrentPreprocessor) { SourceManager &SM = CurrentPreprocessor->getSourceManager(); - // Only check for expectations in other diagnostic locations not - // captured during normal parsing. - // FIXME: This check is currently necessary while synthesized files may - // not have their expected-* directives captured during parsing. These - // cases should be fixed and the following loop replaced with one which - // checks only during a debug build and asserts on a mismatch. + +#ifndef NDEBUG + // In a debug build, scan through any files that may have been missed + // during parsing and issue a fatal error if directives are contained + // within these files. If a fatal error occurs, this suggests that + // this file is being parsed separately from the main file. + HeaderSearch &HS = CurrentPreprocessor->getHeaderSearchInfo(); for (FilesWithDiagnosticsSet::iterator I = FilesWithDiagnostics.begin(), End = FilesWithDiagnostics.end(); I != End; ++I) { const FileEntry *E = SM.getFileEntryForID(*I); - if (!E || !FilesWithDirectives.count(E)) { - if (E) - FilesWithDirectives.insert(E); - FindExpectedDiags(*CurrentPreprocessor, ED, *I); - } + // Don't check files already parsed or those handled as modules. + if (E && (FilesParsedForDirectives.count(E) + || HS.findModuleForHeader(E))) + continue; + + if (findDirectives(*CurrentPreprocessor, *I)) + llvm::report_fatal_error(Twine("-verify directives found after rather" + " than during normal parsing of ", + StringRef(E ? E->getName() : "(unknown)"))); } +#endif // Check that the expected diagnostics occurred. NumErrors += CheckResults(Diags, SM, *Buffer, ED); diff --git a/clang/test/ARCMT/verify.m b/clang/test/ARCMT/verify.m new file mode 100644 index 000000000000..9110fe6efae2 --- /dev/null +++ b/clang/test/ARCMT/verify.m @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -arcmt-check -verify %s +// RUN: %clang_cc1 -arcmt-check -verify %t.invalid 2>&1 | FileCheck %s + +#if 0 +// expected-error {{should be ignored}} +#endif + +#error should not be ignored +// expected-error@-1 {{should not be ignored}} + +// CHECK: error: 'error' diagnostics seen but not expected: +// CHECK-NEXT: (frontend): error reading '{{.*}}verify.m.tmp.invalid' +// CHECK-NEXT: 1 error generated. diff --git a/clang/test/ASTMerge/function.c b/clang/test/ASTMerge/function.c index f97eceed9866..320bca2a36f1 100644 --- a/clang/test/ASTMerge/function.c +++ b/clang/test/ASTMerge/function.c @@ -1,9 +1,15 @@ // RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/function1.c // RUN: %clang_cc1 -emit-pch -o %t.2.ast %S/Inputs/function2.c // RUN: %clang_cc1 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only -verify %s // CHECK: function2.c:3:6: error: external function 'f1' declared with incompatible types in different translation units ('void (Int, double)' vs. 'void (int, float)') // CHECK: function1.c:2:6: note: declared here with type 'void (int, float)' // CHECK: function2.c:5:6: error: external function 'f3' declared with incompatible types in different translation units ('void (int)' vs. 'void (void)') // CHECK: function1.c:4:6: note: declared here with type 'void (void)' // CHECK: 2 errors generated + +// expected-error@3 {{external function 'f1' declared with incompatible types}} +// expected-note@2 {{declared here}} +// expected-error@5 {{external function 'f3' declared with incompatible types}} +// expected-note@4 {{declared here}} diff --git a/clang/test/Frontend/verify.c b/clang/test/Frontend/verify.c index 59a441dc4465..f8d0f4282b4a 100644 --- a/clang/test/Frontend/verify.c +++ b/clang/test/Frontend/verify.c @@ -108,3 +108,18 @@ unexpected b; // expected-error@33 1-1 {{unknown type}} // CHECK5-NEXT: 2 errors generated. #endif +#if 0 +// RUN: %clang_cc1 -verify %t.invalid 2>&1 | FileCheck -check-prefix=CHECK6 %s + +// CHECK6: error: 'error' diagnostics seen but not expected: +// CHECK6-NEXT: (frontend): error reading '{{.*}}verify.c.tmp.invalid' +// CHECK6-NEXT: 1 error generated. + +// RUN: echo -e '//expected-error@2{{1}}\n#error 2' | %clang_cc1 -verify 2>&1 | FileCheck -check-prefix=CHECK7 %s + +// CHECK7: error: 'error' diagnostics expected but not seen: +// CHECK7-NEXT: Line 2 (directive at :1): 1 +// CHECK7-NEXT: error: 'error' diagnostics seen but not expected: +// CHECK7-NEXT: Line 2: 2 +// CHECK7-NEXT: 2 errors generated. +#endif diff --git a/clang/test/Frontend/verify2.c b/clang/test/Frontend/verify2.c new file mode 100644 index 000000000000..a1c797581ed9 --- /dev/null +++ b/clang/test/Frontend/verify2.c @@ -0,0 +1,19 @@ +#if 0 +// RUN: %clang_cc1 -verify %s 2>&1 | FileCheck %s + +// Please note that all comments are inside "#if 0" blocks so that +// VerifyDiagnosticConsumer sees no comments while processing this +// test-case. +#endif + +#include "verify2.h" +#error source + +#if 0 +// expected-error {{should be ignored}} + +// CHECK: error: 'error' diagnostics seen but not expected: +// CHECK-NEXT: Line 1: header +// CHECK-NEXT: Line 10: source +// CHECK-NEXT: 2 errors generated. +#endif diff --git a/clang/test/Frontend/verify2.h b/clang/test/Frontend/verify2.h new file mode 100644 index 000000000000..8acbf6efdfe2 --- /dev/null +++ b/clang/test/Frontend/verify2.h @@ -0,0 +1,5 @@ +#error header + +#if 0 +// expected-error {{should be ignored}} +#endif diff --git a/clang/test/Modules/Inputs/category_right.h b/clang/test/Modules/Inputs/category_right.h index d993b50db4bf..812a84078249 100644 --- a/clang/test/Modules/Inputs/category_right.h +++ b/clang/test/Modules/Inputs/category_right.h @@ -8,5 +8,5 @@ -(void)right2; @end -@interface Foo(Duplicate) // expected-warning {{duplicate definition of category}} +@interface Foo(Duplicate) @end diff --git a/clang/test/Modules/lookup.cpp b/clang/test/Modules/lookup.cpp index 98390355339e..70d210750d94 100644 --- a/clang/test/Modules/lookup.cpp +++ b/clang/test/Modules/lookup.cpp @@ -5,6 +5,8 @@ import lookup_left_cxx; #define IMPORT(X) @__experimental_modules_import X IMPORT(lookup_right_cxx); +// in lookup_left.hpp: expected-warning@3 {{weak identifier 'weak_identifier' never declared}} + void test(int i, float f) { // unqualified lookup f0(&i); diff --git a/clang/test/Modules/objc-categories.m b/clang/test/Modules/objc-categories.m index 340f279adf3f..b26759239dda 100644 --- a/clang/test/Modules/objc-categories.m +++ b/clang/test/Modules/objc-categories.m @@ -12,6 +12,7 @@ // in category_left.h: expected-note {{previous definition}} +// in category_right.h: expected-warning@11 {{duplicate definition of category}} @interface Foo(Source) -(void)source;