nik updated this revision to Diff 200736.
nik marked an inline comment as done.
nik added a comment.
Addressed comments.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61487/new/
https://reviews.llvm.org/D61487
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/plugin/ClangTidyPlugin.cpp
test/clang-tidy/basic.cpp
test/clang-tidy/nolint-plugin.cpp
test/clang-tidy/nolintnextline-plugin.cpp
Index: test/clang-tidy/nolintnextline-plugin.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/nolintnextline-plugin.cpp
@@ -0,0 +1,47 @@
+class A { A(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(for-some-other-check)
+class C { C(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE(*)
+class C1 { C1(int i); };
+
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+class C2 { C2(int i); };
+
+// NOLINTNEXTLINE(google-explicit-constructor)
+class C3 { C3(int i); };
+
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+class C4 { C4(int i); };
+
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class C5 { C5(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
Index: test/clang-tidy/nolint-plugin.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/nolint-plugin.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: static-analyzer
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult' -Wunused-variable -I%S/Inputs/nolint 2>&1 | FileCheck %s
+
+#include "trigger_warning.h"
+void I(int& Out) {
+ int In;
+ A1(In, Out);
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+class A { A(int i); };
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class B { B(int i); }; // NOLINT
+
+class C { C(int i); }; // NOLINT(for-some-other-check)
+// CHECK-DAG: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+class C1 { C1(int i); }; // NOLINT(*)
+
+class C2 { C2(int i); }; // NOLINT(not-closed-bracket-is-treated-as-skip-all
+
+class C3 { C3(int i); }; // NOLINT(google-explicit-constructor)
+
+class C4 { C4(int i); }; // NOLINT(some-check, google-explicit-constructor)
+
+class C5 { C5(int i); }; // NOLINT without-brackets-skip-all, another-check
+
+void f() {
+ int i;
+// CHECK-DAG: :[[@LINE-1]]:7: warning: unused variable 'i' [-Wunused-variable]
+// 31:7: warning: unused variable 'i' [-Wunused-variable]
+// int j; // NOLINT
+// int k; // NOLINT(clang-diagnostic-unused-variable)
+}
+
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-DAG: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
Index: test/clang-tidy/basic.cpp
===================================================================
--- test/clang-tidy/basic.cpp
+++ test/clang-tidy/basic.cpp
@@ -1,4 +1,5 @@
// RUN: clang-tidy %s -checks='-*,llvm-namespace-comment' -- | FileCheck %s
+// RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-tidy -Xclang -plugin-arg-clang-tidy -Xclang -checks='-*,llvm-namespace-comment' 2>&1 | FileCheck %s
namespace i {
}
Index: clang-tidy/plugin/ClangTidyPlugin.cpp
===================================================================
--- clang-tidy/plugin/ClangTidyPlugin.cpp
+++ clang-tidy/plugin/ClangTidyPlugin.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "../ClangTidy.h"
+#include "../ClangTidyDiagnosticConsumer.h"
#include "../ClangTidyForceLinker.h"
#include "../ClangTidyModule.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -19,29 +20,39 @@
/// The core clang tidy plugin action. This just provides the AST consumer and
/// command line flag parsing for using clang-tidy as a clang plugin.
class ClangTidyPluginAction : public PluginASTAction {
- /// Wrapper to grant the context the same lifetime as the action. We use
- /// MultiplexConsumer to avoid writing out all the forwarding methods.
+ /// Wrapper to grant the context and diagnostics engine the same lifetime as
+ /// the action.
+ /// We use MultiplexConsumer to avoid writing out all the forwarding methods.
class WrapConsumer : public MultiplexConsumer {
std::unique_ptr<ClangTidyContext> Context;
+ std::unique_ptr<DiagnosticsEngine> DiagEngine;
public:
WrapConsumer(std::unique_ptr<ClangTidyContext> Context,
+ std::unique_ptr<DiagnosticsEngine> DiagEngine,
std::vector<std::unique_ptr<ASTConsumer>> Consumer)
- : MultiplexConsumer(std::move(Consumer)), Context(std::move(Context)) {}
+ : MultiplexConsumer(std::move(Consumer)), Context(std::move(Context)),
+ DiagEngine(std::move(DiagEngine)) {}
};
public:
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
StringRef File) override {
- // Insert the current diagnostics engine.
- Context->setDiagnosticsEngine(&Compiler.getDiagnostics());
+ // Create and set diagnostics engine
+ auto ExternalDiagEngine = &Compiler.getDiagnostics();
+ auto DiagConsumer =
+ new ClangTidyDiagnosticConsumer(*Context, ExternalDiagEngine);
+ auto DiagEngine = llvm::make_unique<DiagnosticsEngine>(
+ new DiagnosticIDs, new DiagnosticOptions, DiagConsumer);
+ Context->setDiagnosticsEngine(DiagEngine.get());
// Create the AST consumer.
ClangTidyASTConsumerFactory Factory(*Context);
std::vector<std::unique_ptr<ASTConsumer>> Vec;
Vec.push_back(Factory.CreateASTConsumer(Compiler, File));
- return llvm::make_unique<WrapConsumer>(std::move(Context), std::move(Vec));
+ return llvm::make_unique<WrapConsumer>(
+ std::move(Context), std::move(DiagEngine), std::move(Vec));
}
bool ParseArgs(const CompilerInstance &,
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -190,6 +190,23 @@
return AllowEnablingAnalyzerAlphaCheckers;
}
+ struct CustomDiagIdParams {
+ CustomDiagIdParams(DiagnosticIDs::Level Level,
+ const std::string &FormatString)
+ : Level(Level), FormatString(FormatString) {}
+ DiagnosticIDs::Level Level;
+ std::string FormatString;
+ };
+
+ llvm::Optional<CustomDiagIdParams>
+ getCustomDiagIdParamsForId(unsigned DiagnosticID) {
+ llvm::DenseMap<unsigned, CustomDiagIdParams>::const_iterator I =
+ CustomDiagIdParamsByDiagnosticID.find(DiagnosticID);
+ if (I != CustomDiagIdParamsByDiagnosticID.end())
+ return I->second;
+ return llvm::None;
+ }
+
private:
// Writes to Stats.
friend class ClangTidyDiagnosticConsumer;
@@ -209,6 +226,7 @@
std::string CurrentBuildDirectory;
+ llvm::DenseMap<unsigned, CustomDiagIdParams> CustomDiagIdParamsByDiagnosticID;
llvm::DenseMap<unsigned, std::string> CheckNamesByDiagnosticID;
bool Profile;
@@ -242,6 +260,7 @@
class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
public:
ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx,
+ DiagnosticsEngine *ExternalDiagEngine = nullptr,
bool RemoveIncompatibleErrors = true);
// FIXME: The concept of converting between FixItHints and Replacements is
@@ -266,7 +285,10 @@
void checkFilters(SourceLocation Location, const SourceManager &Sources);
bool passesLineFilter(StringRef FileName, unsigned LineNumber) const;
+ void forwardDiagnostic(const Diagnostic &Info);
+
ClangTidyContext &Context;
+ DiagnosticsEngine *ExternalDiagEngine;
bool RemoveIncompatibleErrors;
std::vector<ClangTidyError> Errors;
std::unique_ptr<llvm::Regex> HeaderFilter;
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -200,9 +200,12 @@
StringRef CheckName, SourceLocation Loc, StringRef Description,
DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) {
assert(Loc.isValid());
- unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID(
- Level, (Description + " [" + CheckName + "]").str());
+ auto FormatString = (Description + " [" + CheckName + "]").str();
+ unsigned ID =
+ DiagEngine->getDiagnosticIDs()->getCustomDiagID(Level, FormatString);
CheckNamesByDiagnosticID.try_emplace(ID, CheckName);
+ CustomDiagIdParamsByDiagnosticID.try_emplace(
+ ID, CustomDiagIdParams(Level, FormatString));
return DiagEngine->Report(Loc, ID);
}
@@ -275,8 +278,10 @@
}
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
- ClangTidyContext &Ctx, bool RemoveIncompatibleErrors)
- : Context(Ctx), RemoveIncompatibleErrors(RemoveIncompatibleErrors),
+ ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine,
+ bool RemoveIncompatibleErrors)
+ : Context(Ctx), ExternalDiagEngine(ExternalDiagEngine),
+ RemoveIncompatibleErrors(RemoveIncompatibleErrors),
LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false),
LastErrorWasIgnored(false) {}
@@ -461,6 +466,13 @@
IsWarningAsError);
}
+ // If there is an external diagnostics engine, like in the
+ // ClangTidyPluginAction case, forward the diagnostics to it.
+ if (ExternalDiagEngine) {
+ forwardDiagnostic(Info);
+ return;
+ }
+
ClangTidyDiagnosticRenderer Converter(
Context.getLangOpts(), &Context.DiagEngine->getDiagnosticOptions(),
Errors.back());
@@ -494,6 +506,71 @@
return false;
}
+void ClangTidyDiagnosticConsumer::forwardDiagnostic(const Diagnostic &Info) {
+ // Acquire a diagnostic ID also in the external diagnostics engine.
+ auto IDParams = Context.getCustomDiagIdParamsForId(Info.getID());
+ assert(IDParams);
+ unsigned ExternalID = ExternalDiagEngine->getDiagnosticIDs()->getCustomDiagID(
+ IDParams->Level, IDParams->FormatString);
+
+ // Report with the relevant pieces.
+ // TODO: Simplify this by introducing getters/setters for
+ // DiagnosticsEngine::DiagArgumentsKind and friends so that we can get the data
+ // directly from one Engine and set it in the other?!
+ auto builder = ExternalDiagEngine->Report(Info.getLocation(), ExternalID);
+ for (auto Hint : Info.getFixItHints())
+ builder << Hint;
+ for (auto Range : Info.getRanges())
+ builder << Range;
+ for (unsigned Index = 0; Index < Info.getNumArgs(); ++Index) {
+ DiagnosticsEngine::ArgumentKind kind = Info.getArgKind(Index);
+ switch (kind) {
+ case clang::DiagnosticsEngine::ak_std_string:
+ builder << Info.getArgStdStr(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_c_string:
+ builder << Info.getArgCStr(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_sint:
+ builder << Info.getArgSInt(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_uint:
+ builder << Info.getArgUInt(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_tokenkind:
+ builder << static_cast<tok::TokenKind>(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_identifierinfo:
+ builder << Info.getArgIdentifier(Index);
+ break;
+ case clang::DiagnosticsEngine::ak_qual:
+ builder << Qualifiers::fromOpaqueValue(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_qualtype:
+ builder << QualType::getFromOpaquePtr((void *)Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_declarationname:
+ builder << DeclarationName::getFromOpaqueInteger(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_nameddecl:
+ builder << reinterpret_cast<const NamedDecl *>(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_nestednamespec:
+ builder << reinterpret_cast<NestedNameSpecifier *>(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_declcontext:
+ builder << reinterpret_cast<DeclContext *>(Info.getRawArg(Index));
+ break;
+ case clang::DiagnosticsEngine::ak_qualtype_pair:
+ assert(false); // This one is not passed around.
+ break;
+ case clang::DiagnosticsEngine::ak_attr:
+ builder << reinterpret_cast<Attr *>(Info.getRawArg(Index));
+ break;
+ }
+ }
+}
+
void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location,
const SourceManager &Sources) {
// Invalid location may mean a diagnostic in a command line, don't skip these.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
