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 @@ -185,6 +185,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; @@ -197,6 +201,8 @@ class CachedGlobList; std::unique_ptr CheckFilter; std::unique_ptr WarningAsErrorFilter; + // Cache for compiled regex with HeaderFilter. + mutable std::unique_ptr HeaderFilter; LangOptions LangOpts; @@ -259,10 +265,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); @@ -275,7 +277,6 @@ bool RemoveIncompatibleErrors; bool GetFixesFromNotes; 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 @@ -276,6 +276,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, bool GetFixesFromNotes) @@ -364,6 +371,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, @@ -373,6 +415,9 @@ return true; if (!Loc.isMacroID()) return false; + if (Context.getOptions().FilterMacroFromHeaders.getValueOr(false) && + isErrorFromMacroInIgnoredHeader(SM, Loc, Context)) + return true; Loc = SM.getImmediateExpansionRange(Loc).getBegin(); } return false; @@ -601,20 +646,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/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h @@ -79,6 +79,10 @@ /// Output warnings from system headers matching \c HeaderFilterRegex. llvm::Optional SystemHeaders; + /// Suppress diagnostics from macro expansion if the macro is defined in a + /// header not allowed \c HeaderFilterRegex. + llvm::Optional FilterMacroFromHeaders; + /// Format code around applied fixes with clang-format using this /// style. /// diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp @@ -89,6 +89,7 @@ IO.mapOptional("Checks", Options.Checks); IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors); IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex); + IO.mapOptional("FilterMacroFromHeaders", Options.FilterMacroFromHeaders); IO.mapOptional("AnalyzeTemporaryDtors", Ignored); // legacy compatibility IO.mapOptional("FormatStyle", Options.FormatStyle); IO.mapOptional("User", Options.User); @@ -112,6 +113,7 @@ Options.WarningsAsErrors = ""; Options.HeaderFilterRegex = ""; Options.SystemHeaders = false; + Options.FilterMacroFromHeaders = false; Options.FormatStyle = "none"; Options.User = llvm::None; for (const ClangTidyModuleRegistry::entry &Module : @@ -148,6 +150,7 @@ mergeCommaSeparatedLists(WarningsAsErrors, Other.WarningsAsErrors); overrideValue(HeaderFilterRegex, Other.HeaderFilterRegex); overrideValue(SystemHeaders, Other.SystemHeaders); + overrideValue(FilterMacroFromHeaders, Other.FilterMacroFromHeaders); overrideValue(FormatStyle, Other.FormatStyle); overrideValue(User, Other.User); overrideValue(UseColor, Other.UseColor); diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -43,12 +43,13 @@ $ clang-tidy -dump-config --- - Checks: '-*,some-check' - WarningsAsErrors: '' - HeaderFilterRegex: '' - FormatStyle: none - InheritParentConfig: true - User: user + Checks: '-*,some-check' + WarningsAsErrors: '' + HeaderFilterRegex: '' + FilterMacroFromHeaders: true + FormatStyle: none + InheritParentConfig: true + User: user CheckOptions: - key: some-check.SomeOption value: 'some value' @@ -112,6 +113,13 @@ cl::init(""), cl::cat(ClangTidyCategory)); +static cl::opt + FilterMacroFromHeaders("filter-macro-from-headers", + cl::desc(R"(Suppress diagnostics from macro expansion +if the macro is defined in a header that is +not allowed in -header-filter.)"), + cl::init(false), cl::cat(ClangTidyCategory)); + static cl::opt Fix("fix", cl::desc(R"( Apply suggested fixes. Without -fix-errors clang-tidy will bail out if any compilation @@ -128,10 +136,10 @@ cl::init(false), cl::cat(ClangTidyCategory)); static cl::opt FixNotes("fix-notes", cl::desc(R"( -If a warning has no fix, but a single fix can -be found through an associated diagnostic note, -apply the fix. -Specifying this flag will implicitly enable the +If a warning has no fix, but a single fix can +be found through an associated diagnostic note, +apply the fix. +Specifying this flag will implicitly enable the '--fix' flag. )"), cl::init(false), cl::cat(ClangTidyCategory)); @@ -301,6 +309,7 @@ DefaultOptions.WarningsAsErrors = ""; DefaultOptions.HeaderFilterRegex = HeaderFilter; DefaultOptions.SystemHeaders = SystemHeaders; + DefaultOptions.FilterMacroFromHeaders = FilterMacroFromHeaders; DefaultOptions.FormatStyle = FormatStyle; DefaultOptions.User = llvm::sys::Process::GetEnv("USER"); // USERNAME is used on Windows. @@ -316,6 +325,8 @@ OverrideOptions.HeaderFilterRegex = HeaderFilter; if (SystemHeaders.getNumOccurrences() > 0) OverrideOptions.SystemHeaders = SystemHeaders; + if (FilterMacroFromHeaders.getNumOccurrences() > 0) + OverrideOptions.FilterMacroFromHeaders = FilterMacroFromHeaders; if (FormatStyle.getNumOccurrences() > 0) OverrideOptions.FormatStyle = FormatStyle; if (UseColor.getNumOccurrences() > 0) 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,42 @@ -// 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' --filter-macro-from-headers %s -- | FileCheck %s +// RUN: clang-tidy --config='{"Checks": "-*,google-explicit-constructor,modernize-avoid-c-arrays", "HeaderFilterRegex": "macros_filter.h", "FilterMacroFromHeaders": true}' %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]