xgsa updated this revision to Diff 128091.
xgsa marked an inline comment as done.
xgsa added a comment.
Review comments applied.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41326
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
test/clang-tidy/nolint-usage.cpp
Index: test/clang-tidy/nolint-usage.cpp
===================================================================
--- test/clang-tidy/nolint-usage.cpp
+++ test/clang-tidy/nolint-usage.cpp
@@ -22,19 +22,19 @@
// Case: NO_LINT for unknown check on line with an error on some check.
class A6 { A6(int i); }; // NOLINT(unknown-check)
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
-// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: unknown check name 'unknown-check' is specified for NOLINT comment
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: unknown check name 'unknown-check' specified in NOLINT comment
// Case: NO_LINT for unknown check on line without errors.
class A7 { explicit A7(int i); }; // NOLINT(unknown-check)
-// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: unknown check name 'unknown-check' is specified for NOLINT comment
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: unknown check name 'unknown-check' specified in NOLINT comment
// Case: NO_LINT with not closed parenthesis without check names.
class A8 { A8(int i); }; // NOLINT(
-// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: NOLINT comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: unbalanced parentheses in NOLINT comment; all diagnostics silenced for this line
// Case: NO_LINT with not closed parenthesis with check names.
class A9 { A9(int i); }; // NOLINT(unknown-check
-// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: NOLINT comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: unbalanced parentheses in NOLINT comment; all diagnostics silenced for this line
// Case: NO_LINT with clang diagnostics
int f() {
@@ -102,23 +102,23 @@
// Case: NO_LINT_NEXT_LINE for unknown check before line with an error on some check.
// NOLINTNEXTLINE(unknown-check)
class C6 { C6(int i); };
-// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: unknown check name 'unknown-check' is specified for NOLINTNEXTLINE comment
+// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: unknown check name 'unknown-check' specified in NOLINTNEXTLINE comment
// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: single-argument constructors must be marked explicit
// Case: NO_LINT_NEXT_LINE for unknown check before line without errors.
// NOLINTNEXTLINE(unknown-check)
class C7 { explicit C7(int i); };
-// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: unknown check name 'unknown-check' is specified for NOLINTNEXTLINE comment
+// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: unknown check name 'unknown-check' specified in NOLINTNEXTLINE comment
// Case: NO_LINT_NEXT_LINE with not closed parenthesis without check names.
// NOLINTNEXTLINE(
class C8 { C8(int i); };
-// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: NOLINTNEXTLINE comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line
+// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: unbalanced parentheses in NOLINTNEXTLINE comment; all diagnostics silenced for this line
// Case: NO_LINT_NEXT_LINE with not closed parenthesis with check names.
// NOLINTNEXTLINE(unknown-check
class C9 { C9(int i); };
-// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: NOLINTNEXTLINE comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line
+// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: unbalanced parentheses in NOLINTNEXTLINE comment; all diagnostics silenced for this line
// Case: NO_LINT_NEXT_LINE with clang diagnostics
int g() {
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -27,6 +27,7 @@
class ASTContext;
class CompilerInstance;
class Preprocessor;
+
namespace ast_matchers {
class MatchFinder;
}
@@ -202,7 +203,7 @@
}
private:
- // Calls setDiagnosticsEngine(), storeError() and getNolintCollector().
+ // Calls setDiagnosticsEngine(), storeError(), and getNolintCollector().
friend class ClangTidyDiagnosticConsumer;
friend class ClangTidyPluginAction;
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -16,15 +16,16 @@
///
//===----------------------------------------------------------------------===//
-#include "ClangTidyDiagnosticConsumer.h"
-#include "ClangTidyOptions.h"
#include "ClangTidy.h"
+#include "ClangTidyDiagnosticConsumer.h"
#include "ClangTidyModule.h"
+#include "ClangTidyOptions.h"
#include "clang/AST/ASTDiagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Frontend/DiagnosticRenderer.h"
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/iterator_range.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
@@ -129,7 +130,7 @@
// Check if the specific checks are specified in brackets.
if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
++BracketIndex;
- const size_t BracketEndIndex = Line.find(')', BracketIndex);
+ size_t BracketEndIndex = Line.find(')', BracketIndex);
if ((ClosingBracketFound = BracketEndIndex != StringRef::npos)) {
StringRef ChecksStr =
Line.substr(BracketIndex, BracketEndIndex - BracketIndex);
@@ -158,10 +159,10 @@
return CheckNames.empty();
}
- const SmallVector<StringRef, 1>& getCheckNames() const {
+ llvm::iterator_range<SmallVectorImpl<StringRef>::const_iterator> getCheckNames() const {
assert(isNolintFound());
assert(!isAppliedToAllChecks());
- return CheckNames;
+ return llvm::make_range(CheckNames.begin(), CheckNames.end());
}
bool isClosingBracketFound() const {
@@ -258,19 +259,19 @@
private:
struct NolintTargetInfoHasher {
- size_t operator() (const NolintTargetInfo &i) const {
- return llvm::hash_combine(i.LineNumber, i.CheckName);
+ size_t operator()(const NolintTargetInfo &Info) const {
+ return llvm::hash_combine(Info.LineNumber, Info.CheckName);
}
};
struct NolintTargetInfoEq {
- bool operator()(const NolintTargetInfo &i1,
- const NolintTargetInfo &i2) const {
- return i1.LineNumber == i2.LineNumber && i1.CheckName == i2.CheckName;
+ bool operator()(const NolintTargetInfo &Info1,
+ const NolintTargetInfo &Info2) const {
+ return Info1.LineNumber == Info2.LineNumber && Info1.CheckName == Info2.CheckName;
}
};
public:
- NolintCommentsCollector(ClangTidyContext *Context)
+ explicit NolintCommentsCollector(ClangTidyContext *Context)
: Context(Context) {}
using NolintMap = std::unordered_map<NolintTargetInfo,
@@ -313,17 +314,17 @@
if (!CheckName.empty() && !Context->hasCheck(CheckName) &&
!CheckName.startswith(ClangDiagnosticsPrefix)) {
Context->diag(NolintCheckName, NolintLoc,
- "unknown check name '%0' is specified for %1 comment")
+ "unknown check name '%0' specified in %1 comment")
<< CheckName << CommentName;
} else {
NolintComments.emplace(NolintTargetInfo{LineNo, CheckName},
NolintCommentInfo{NolintLoc, CommentType});
}
}
if (!NolintParser.isClosingBracketFound()) {
Context->diag(NolintCheckName, NolintLoc,
- "%0 comment has an opening parenthesis and no closing one, "
- "so all the diagnostics will be silenced for the line")
+ "unbalanced parentheses in %0 comment; all diagnostics silenced "
+ "for this line")
<< CommentName;
}
}
@@ -378,9 +379,9 @@
LangOpts = Context->getLangOpts();
}
-void ClangTidyContext::setPreprocessor(Preprocessor *preprocessor) {
- if (preprocessor && isCheckEnabled(NolintCheckName)) {
- preprocessor->addCommentHandler(NolintCollector.get());
+void ClangTidyContext::setPreprocessor(Preprocessor *Preprocessor) {
+ if (Preprocessor && isCheckEnabled(NolintCheckName)) {
+ Preprocessor->addCommentHandler(NolintCollector.get());
}
}
@@ -421,7 +422,7 @@
}
// This method is not const, because it fills cache on first demand.
-// It is possible to fill cache in constructor or make cache volatile
+// It is possible to fill cache in constructor or to make cache mutable
// and make this method const, but they seem like worse solutions.
bool ClangTidyContext::hasCheck(StringRef CheckName) {
if (AllCheckNamesCache.empty()) {
@@ -492,7 +493,7 @@
// otherwise there will never be diagnostics on "// NOLINT".
return CheckName != NolintCheckName;
}
- const SmallVector<StringRef, 1>& CheckNames = NolintParser.getCheckNames();
+ const auto &CheckNames = NolintParser.getCheckNames();
return llvm::find(CheckNames, CheckName) != CheckNames.end();
}
@@ -563,7 +564,7 @@
SourceLocation DiagLocation = Info.getLocation();
if (DiagLocation.isValid()) {
// This is necessary for finding redundant NOLINT comments. Fill collection
- // here, as there won't be another change if diagnostics is filtered.
+ // here, as there won't be another chance if diagnostics are filtered.
DiagIDsToLineNumbers[Info.getID()].push_back(
Info.getSourceManager().getExpansionLineNumber(DiagLocation));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
