DmitryPolukhin updated this revision to Diff 303121.
DmitryPolukhin added a comment.
Fix clang-tidy warning
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/cppcoreguidelines/ProTypeVarargCheck.cpp
clang-tools-extra/test/clang-tidy/infrastructure/macros.cpp
clang-tools-extra/test/clang-tidy/infrastructure/macros.h
clang-tools-extra/test/clang-tidy/infrastructure/macros_filter.h
Index: clang-tools-extra/test/clang-tidy/infrastructure/macros_filter.h
===================================================================
--- /dev/null
+++ 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]
Index: clang-tools-extra/test/clang-tidy/infrastructure/macros.h
===================================================================
--- /dev/null
+++ 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
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,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:
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
@@ -115,7 +115,10 @@
}
if (const auto *Matched = Result.Nodes.getNodeAs<Expr>("va_use")) {
- diag(Matched->getExprLoc(),
+ // va_arg is a macro defined in system header but diags from such macro
+ // are not always reported so use expansion location instead to always
+ // report
+ diag(Result.SourceManager->getExpansionLoc(Matched->getExprLoc()),
"do not use va_arg to define c-style vararg functions; "
"use variadic templates instead");
}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ 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<CachedGlobList> CheckFilter;
std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
+ // Cache for compiled regex with HeaderFilter.
+ mutable std::unique_ptr<llvm::Regex> 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<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
@@ -245,6 +245,13 @@
return "";
}
+llvm::Regex *ClangTidyContext::getHeaderFilter() const {
+ if (!HeaderFilter)
+ HeaderFilter =
+ std::make_unique<llvm::Regex>(*getOptions().HeaderFilterRegex);
+ 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;
+
+ // Skip macro from system headers.
+ if (!*Context.getOptions().SystemHeaders && SM.isInSystemHeader(SpellingLoc))
+ return true;
+
+ 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<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
