Index: tools/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h ================================================================== --- tools/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h +++ tools/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h @@ -164,21 +164,26 @@ llvm::DeleteContainerPointers(Warnings); llvm::DeleteContainerPointers(Notes); } }; -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(); public: /// Create a new verifying diagnostic client, which will issue errors to @@ -189,10 +194,17 @@ virtual void BeginSourceFile(const LangOptions &LangOpts, const Preprocessor *PP); 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, const Diagnostic &Info); Index: tools/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp ================================================================== --- tools/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp +++ tools/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp @@ -13,10 +13,11 @@ #include "clang/Basic/FileManager.h" #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" #include "llvm/Support/raw_ostream.h" @@ -24,50 +25,91 @@ typedef VerifyDiagnosticConsumer::Directive Directive; 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) { + CurrentPreprocessor = PP; + if (PP) { +#ifndef NDEBUG + VerifyFileTracker *V = new VerifyFileTracker(FilesParsedForDirectives, + PP->getSourceManager()); + const_cast(PP)->addPPCallbacks(V); +#endif + const_cast(PP)->addCommentHandler(this); + } + } + assert(CurrentPreprocessor == PP && "Preprocessor changed!"); PrimaryClient->BeginSourceFile(LangOpts, PP); } void VerifyDiagnosticConsumer::EndSourceFile() { - if (CurrentPreprocessor) - const_cast(CurrentPreprocessor)->removeCommentHandler(this); + assert(ActiveSourceFiles && "No active source files!"); + PrimaryClient->EndSourceFile(); CheckDiagnostics(); - PrimaryClient->EndSourceFile(); - - CurrentPreprocessor = 0; + // Detach comment handler once last active source file completed. + if (--ActiveSourceFiles == 0 && CurrentPreprocessor) { + const_cast(CurrentPreprocessor)->removeCommentHandler(this); + 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())); } +#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); } @@ -190,11 +231,11 @@ /// ParseDirective - Go through the comment and see if it indicates expected /// 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; for (ParseHelper PH(S); !PH.Done();) { // Search for token: expected @@ -208,18 +249,23 @@ PH.Advance(); // 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"; @@ -358,13 +404,11 @@ return false; // 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; } std::string C2; C2.reserve(C.size()); @@ -390,23 +434,24 @@ C2 += '\\'; } } 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. const llvm::MemoryBuffer *FromFile = SM.getBuffer(FID); Lexer RawLex(FID, FromFile, SM, PP.getLangOpts()); @@ -414,21 +459,25 @@ // Return comments as tokens, this is how we find expected diagnostics. RawLex.SetCommentRetentionState(true); Token Tok; Tok.setKind(tok::comment); + bool Found = false; while (Tok.isNot(tok::eof)) { RawLex.Lex(Tok); if (!Tok.is(tok::comment)) continue; std::string Comment = PP.getSpelling(Tok); 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. static unsigned PrintUnexpected(DiagnosticsEngine &Diags, SourceManager *SourceMgr, const_diag_iterator diag_begin, @@ -554,26 +603,32 @@ // If we have a preprocessor, scan the source for expected diagnostic // 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); } else { NumErrors += (PrintUnexpected(Diags, 0, Buffer->err_begin(), ADDED tools/clang/test/Frontend/verify2.c Index: tools/clang/test/Frontend/verify2.c ================================================================== --- tools/clang/test/Frontend/verify2.c +++ tools/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 ADDED tools/clang/test/Frontend/verify2.h Index: tools/clang/test/Frontend/verify2.h ================================================================== --- tools/clang/test/Frontend/verify2.h +++ tools/clang/test/Frontend/verify2.h @@ -0,0 +1,5 @@ +#error header + +#if 0 +// expected-error {{should be ignored}} +#endif