diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -177,6 +177,10 @@ DiagEngine->getDiagnosticIDs()->getDescription(DiagnosticID))); } + /// Returns the \c HeaderFilter constructed for the options set in the + /// context. + llvm::Regex *getHeaderFilter() const; + private: // Writes to Stats. friend class ClangTidyDiagnosticConsumer; @@ -189,6 +193,8 @@ class CachedGlobList; std::unique_ptr CheckFilter; std::unique_ptr WarningAsErrorFilter; + // Cache for compiled regex with HeaderFilter. + mutable std::unique_ptr HeaderFilter; LangOptions LangOpts; @@ -243,10 +249,6 @@ void removeIncompatibleErrors(); void removeDuplicatedDiagnosticsOfAliasCheckers(); - /// Returns the \c HeaderFilter constructed for the options set in the - /// context. - llvm::Regex *getHeaderFilter(); - /// Updates \c LastErrorRelatesToUserCode and LastErrorPassesLineFilter /// according to the diagnostic \p Location. void checkFilters(SourceLocation Location, const SourceManager &Sources); @@ -258,7 +260,6 @@ DiagnosticsEngine *ExternalDiagEngine; bool RemoveIncompatibleErrors; std::vector Errors; - std::unique_ptr HeaderFilter; bool LastErrorRelatesToUserCode; bool LastErrorPassesLineFilter; bool LastErrorWasIgnored; diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -245,6 +245,13 @@ return ""; } +llvm::Regex *ClangTidyContext::getHeaderFilter() const { + if (!HeaderFilter) + HeaderFilter = std::make_unique( + getOptions().HeaderFilterRegex.getValueOr("")); + return HeaderFilter.get(); +} + ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer( ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine, bool RemoveIncompatibleErrors) @@ -331,6 +338,41 @@ return IsNOLINTFound("NOLINTNEXTLINE", PrevLine, DiagID, Context); } +static bool isErrorFromMacroInIgnoredHeader(const SourceManager &SM, + SourceLocation Loc, + const ClangTidyContext &Context) { + SourceLocation SpellingLoc = SM.getImmediateSpellingLoc(Loc); + // Don't skip macro from the main file. + if (SM.isInMainFile(SpellingLoc)) + return false; + + // Don't skip macro from system headers to report issues on `NULL`, ObjC + // `YES/NO` and similar system macro. + if (SM.isInSystemHeader(SpellingLoc)) + return false; + + FileID FID = SM.getDecomposedExpansionLoc(SpellingLoc).first; + const FileEntry *File = SM.getFileEntryForID(FID); + + // -DMACRO definitions on the command line have locations in a virtual buffer + // that doesn't have a FileEntry. Don't skip these. + if (!File) + return false; + + StringRef FileName(File->getName()); + if (const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID())) { + if (MainFile->getName() == FileName) { + // This check should be equvalent to isInMainFile but when clangd loads + // preamble isInMainFile gives wrong result for macro spelling location. + // It looks like the location comes from file included into itself at + // position 1:1. This check fixes ClangdTests/DiagnosticsTest.ClangTidy + // TODO: find out what clangd does wrong with location + return false; + } + } + return !Context.getHeaderFilter()->match(FileName); +} + static bool LineIsMarkedWithNOLINTinMacro(const SourceManager &SM, SourceLocation Loc, unsigned DiagID, const ClangTidyContext &Context, @@ -340,6 +382,8 @@ return true; if (!Loc.isMacroID()) return false; + if (isErrorFromMacroInIgnoredHeader(SM, Loc, Context)) + return true; Loc = SM.getImmediateExpansionRange(Loc).getBegin(); } return false; @@ -550,20 +594,13 @@ StringRef FileName(File->getName()); LastErrorRelatesToUserCode = LastErrorRelatesToUserCode || Sources.isInMainFile(Location) || - getHeaderFilter()->match(FileName); + Context.getHeaderFilter()->match(FileName); unsigned LineNumber = Sources.getExpansionLineNumber(Location); LastErrorPassesLineFilter = LastErrorPassesLineFilter || passesLineFilter(FileName, LineNumber); } -llvm::Regex *ClangTidyDiagnosticConsumer::getHeaderFilter() { - if (!HeaderFilter) - HeaderFilter = - std::make_unique(*Context.getOptions().HeaderFilterRegex); - return HeaderFilter.get(); -} - void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() { // Each error is modelled as the set of intervals in which it applies // replacements. To detect overlapping replacements, we use a sweep line diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/macros.h b/clang-tools-extra/test/clang-tidy/infrastructure/macros.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/macros.h @@ -0,0 +1,9 @@ +#define HEADER_MACRO_DIAG_IN_ARG(a) a +#define HEADER_MACRO_DIAG_IN_BODY int var_C1[10] +#define HEADER_MACRO_DIAG_IN_BODY2 int var_C2[10] + +#define DEFINE_bool(name, val) \ + namespace fLB { \ + typedef char FLAG_##name##_value_is_not_a_bool[(sizeof(val) != sizeof(bool)) ? -1 : 1]; \ + } \ + bool FLAG_##name = val diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/macros.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/macros.cpp --- a/clang-tools-extra/test/clang-tidy/infrastructure/macros.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/macros.cpp @@ -1,7 +1,41 @@ -// RUN: clang-tidy -checks='-*,google-explicit-constructor' %s -- | FileCheck %s +// RUN: clang-tidy -checks='-*,google-explicit-constructor,modernize-avoid-c-arrays' --header-filter='macros_filter.h' %s -- | FileCheck %s + +#include "macros.h" +#include "macros_filter.h" #define Q(name) class name { name(int i); } Q(A); // CHECK: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit -// CHECK: :3:30: note: expanded from macro 'Q' +// CHECK: :[[@LINE-4]]:30: note: expanded from macro 'Q' + +#define MAIN_MACRO_DIAG_IN_ARG(a) a +MAIN_MACRO_DIAG_IN_ARG(int var_A[10]); +// CHECK: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> instead + +#define MAIN_MACRO_DIAG_IN_BODY int var_A1[10] +MAIN_MACRO_DIAG_IN_BODY; +// CHECK: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead +// CHECK: :[[@LINE-3]]:33: note: expanded from macro 'MAIN_MACRO_DIAG_IN_BODY' + +HEADER_FILTER_MACRO_DIAG_IN_ARG(int var_B[10]); +// CHECK: :[[@LINE-1]]:33: warning: do not declare C-style arrays, use std::array<> instead + +HEADER_FILTER_MACRO_DIAG_IN_BODY; +// CHECK: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead +// CHECK: note: expanded from macro 'HEADER_FILTER_MACRO_DIAG_IN_BODY' + +#define MAIN_MACRO_WRAPPER HEADER_FILTER_MACRO_DIAG_IN_BODY2 +MAIN_MACRO_WRAPPER; +// CHECK: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead +// CHECK: note: expanded from macro 'MAIN_MACRO_WRAPPER' +// CHECK: note: expanded from macro 'HEADER_FILTER_MACRO_DIAG_IN_BODY2' + +HEADER_MACRO_DIAG_IN_ARG(int var_C[10]); +HEADER_MACRO_DIAG_IN_BODY; + +#define MAIN_MACRO_WRAPPER2 HEADER_MACRO_DIAG_IN_BODY2 +MAIN_MACRO_WRAPPER2; + +DEFINE_bool(test, false); +// CHECK-NOT: warning: diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/macros_filter.h b/clang-tools-extra/test/clang-tidy/infrastructure/macros_filter.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/macros_filter.h @@ -0,0 +1,3 @@ +#define HEADER_FILTER_MACRO_DIAG_IN_ARG(a) a +#define HEADER_FILTER_MACRO_DIAG_IN_BODY int var_B1[10] +#define HEADER_FILTER_MACRO_DIAG_IN_BODY2 int var_B2[10]