DmitryPolukhin updated this revision to Diff 347968.
DmitryPolukhin added a comment.
Herald added a subscriber: dexonsmith.
Added test for system like object macro
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90835/new/
https://reviews.llvm.org/D90835
Files:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
clang-tools-extra/clang-tidy/ClangTidyOptions.h
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/macros/macros.h
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/macros/macros_filter.h
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/macros/system/sysmacros.h
clang-tools-extra/test/clang-tidy/infrastructure/macros.cpp
clang/include/clang/Basic/SourceManager.h
Index: clang/include/clang/Basic/SourceManager.h
===================================================================
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -397,6 +397,11 @@
getExpansionLocStart() != getExpansionLocEnd();
}
+ bool isObjectMacroExpansion() const {
+ return getExpansionLocStart().isValid() &&
+ getExpansionLocStart() == getExpansionLocEnd();
+ }
+
/// Return a ExpansionInfo for an expansion.
///
/// Start and End specify the expansion range (where the macro is
Index: clang-tools-extra/test/clang-tidy/infrastructure/macros.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/macros.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/macros.cpp
@@ -1,7 +1,50 @@
-// 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 -- -isystem %S/Inputs/macros/system -I %S/Inputs/macros | FileCheck %s
+// RUN: clang-tidy --config='{"Checks": "-*,google-explicit-constructor,modernize-avoid-c-arrays", "HeaderFilterRegex": "macros_filter.h", "FilterMacroFromHeaders": true}' %s -- -isystem %S/Inputs/macros/system -I %S/Inputs/macros | FileCheck %s
+
+#include "macros.h"
+#include "macros_filter.h"
+#include <sysmacros.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'
+
+// CHECK-NOT: warning:
+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;
+
+SYS_HEADER_MACRO_FUNCTION(int var_D[10]);
+
+SYS_HEADER_MACRO_OBJECT var_D1[10];
+// CHECK: :[[@LINE-1]]:1: warning: do not declare C-style arrays, use std::array<> instead
+// CHECK: note: expanded from macro 'SYS_HEADER_MACRO_OBJECT'
+
+DEFINE_bool(test, false);
+// CHECK-NOT: warning:
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/macros/system/sysmacros.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/macros/system/sysmacros.h
@@ -0,0 +1,2 @@
+#define SYS_HEADER_MACRO_FUNCTION(a) a
+#define SYS_HEADER_MACRO_OBJECT int
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/macros/macros_filter.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/macros/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]
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/macros/macros.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/macros/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
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ 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<bool>
+ 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<bool> 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<bool> 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)
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -79,6 +79,10 @@
/// Output warnings from system headers matching \c HeaderFilterRegex.
llvm::Optional<bool> SystemHeaders;
+ /// Suppress diagnostics from macro expansion if the macro is defined in a
+ /// header not allowed \c HeaderFilterRegex.
+ llvm::Optional<bool> FilterMacroFromHeaders;
+
/// Format code around applied fixes with clang-format using this
/// style.
///
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ 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);
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ 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<CachedGlobList> CheckFilter;
std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
+ // Cache for compiled regex with HeaderFilter.
+ mutable std::unique_ptr<llvm::Regex> 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<ClangTidyError> Errors;
- std::unique_ptr<llvm::Regex> HeaderFilter;
bool LastErrorRelatesToUserCode;
bool LastErrorPassesLineFilter;
bool LastErrorWasIgnored;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -276,6 +276,13 @@
return "";
}
+llvm::Regex *ClangTidyContext::getHeaderFilter() const {
+ if (!HeaderFilter)
+ HeaderFilter = std::make_unique<llvm::Regex>(
+ getOptions().HeaderFilterRegex.getValueOr(""));
+ return HeaderFilter.get();
+}
+
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine,
bool RemoveIncompatibleErrors, bool GetFixesFromNotes)
@@ -364,6 +371,44 @@
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 object-like system macro.
+ assert(Loc.isMacroID() && "Not a macro expansion loc!");
+ const SrcMgr::ExpansionInfo &Expansion =
+ SM.getSLocEntry(SM.getFileID(Loc)).getExpansion();
+ if (SM.isInSystemHeader(SpellingLoc) && Expansion.isObjectMacroExpansion())
+ 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 +418,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;
@@ -607,20 +655,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<llvm::Regex>(*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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
