-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[clang-tidy] Add ClangQueryChecks config option #123734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
//===--- ClangQueryCheck.cpp - clang-tidy ----------------------------===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#include "ClangQueryCheck.h" | ||
#include "clang/ASTMatchers/ASTMatchFinder.h" | ||
|
||
using namespace clang::ast_matchers; | ||
|
||
namespace clang::tidy::misc { | ||
|
||
void ClangQueryCheck::registerMatchers(MatchFinder *Finder) { | ||
for (const auto &Matcher : Matchers) { | ||
bool Ok = Finder->addDynamicMatcher(Matcher, this); | ||
assert(Ok && "Expected to get top level matcher from query parser"); | ||
} | ||
} | ||
|
||
void ClangQueryCheck::check(const MatchFinder::MatchResult &Result) { | ||
auto Map = Result.Nodes.getMap(); | ||
for (const auto &[k, v] : Map) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. avoid to use k, v and map. it is hard to understand what the meaning of this variable. |
||
diag(v.getSourceRange().getBegin(), k) << v.getSourceRange(); | ||
} | ||
} | ||
|
||
} // namespace clang::tidy::misc |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
//===--- ClangQueryCheck.h - clang-tidy --------------------*- C++ -*-===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGQUERYCHECK_H | ||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGQUERYCHECK_H | ||
|
||
#include "ClangTidyCheck.h" | ||
#include "clang/ASTMatchers/Dynamic/VariantValue.h" | ||
#include <vector> | ||
|
||
namespace clang::query { | ||
class QuerySession; | ||
} // namespace clang::query | ||
|
||
namespace clang::tidy::misc { | ||
|
||
/// A check that matches a given matchers printing their binds as warnings | ||
class ClangQueryCheck : public ClangTidyCheck { | ||
using MatcherVec = std::vector<ast_matchers::dynamic::DynTypedMatcher>; | ||
|
||
public: | ||
ClangQueryCheck(StringRef Name, ClangTidyContext *Context, | ||
MatcherVec Matchers) | ||
: ClangTidyCheck(Name, Context), Matchers(std::move(Matchers)) {} | ||
|
||
void registerMatchers(ast_matchers::MatchFinder *Finder) override; | ||
void check(const ast_matchers::MatchFinder::MatchResult &Result) override; | ||
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { | ||
return LangOpts.CPlusPlus; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unnecessary limitation, maybe this should be an "config option" |
||
} | ||
|
||
private: | ||
MatcherVec Matchers; | ||
}; | ||
|
||
} // namespace clang::tidy::misc | ||
|
||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGQUERYCHECK_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,6 +140,9 @@ Improvements to clang-tidy | |
:doc:`readability-redundant-access-specifiers <clang-tidy/checks/readability/redundant-access-specifiers>`, CheckFirstDeclaration | ||
:doc:`readability-redundant-casting <clang-tidy/checks/readability/redundant-casting>`, IgnoreTypeAliases | ||
|
||
- New :program:`clang-tidy` config property `ClangQueryChecks` that allows adding | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. merge with release notes in line 120 |
||
custom checks based on a :program:`clang-query` syntax | ||
|
||
New checks | ||
^^^^^^^^^^ | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -292,6 +292,18 @@ An overview of all the command-line options: | |
Checks - Same as '--checks'. Additionally, the list of | ||
globs can be specified as a list instead of a | ||
string. | ||
ClangQueryChecks - List of key-value pairs. Key specifies a name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be documented in separate section, thats not a good place for example |
||
of the new check and value specifies a list | ||
of matchers in the form of clang-query | ||
syntax. Example: | ||
ClangQueryChecks: | ||
custom-check: | | ||
let matcher varDecl( | ||
hasTypeLoc( | ||
typeLoc().bind("Custom message") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if i want to get "name of fuction" in output message ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is possible to get with transformers. There are some examples that construct fixes and messages out of binds in documentation llvm::StringRef s = "str";
makeRule(
cxxMemberCallExpr(
on(expr(hasType(namedDecl(hasName("std::string"))))
.bind(s)),
callee(cxxMethodDecl(hasName("size")))),
changeTo(cat("Size(", node(s), ")")),
cat("Method ``size`` is deprecated in favor of free function ``Size``")); But a parser for them is not implemented
I didn't thought about that. Currently it should work but will print all binds.
It will be possible with transformers too makeRule(returnStmt(),
note(node(RootID), cat("some note")),
cat("message")), I think that currently matchers can be left as a basic system that will only generate warnings from the strings inside binds. And later this feature can be expanded with transformers |
||
) | ||
) | ||
match matcher | ||
ExcludeHeaderFilterRegex - Same as '--exclude-header-filter'. | ||
ExtraArgs - Same as '--extra-arg'. | ||
ExtraArgsBefore - Same as '--extra-arg-before'. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
// DEFINE: %{custom-call-yaml} = custom-call: 'm callExpr().bind(\"Custom message\")' | ||
// | ||
// DEFINE: %{custom-let-call-yaml} = custom-let-call: \" \ | ||
// DEFINE: let expr varDecl( \ | ||
// DEFINE: hasType(asString(\\\"long long\\\")), \ | ||
// DEFINE: hasTypeLoc(typeLoc().bind(\\\"Let message\\\")) \ | ||
// DEFINE: ) \n \ | ||
// DEFINE: match expr\" | ||
// | ||
// DEFINE: %{full-config} = "{ClangQueryChecks: {%{custom-call-yaml},%{custom-let-call-yaml}}}" | ||
|
||
//Check single match expression | ||
// RUN: clang-tidy %s -checks='-*, custom-*' \ | ||
// RUN: -config="{ClangQueryChecks: {%{custom-call-yaml}}}" \ | ||
// RUN: -- | FileCheck %s -check-prefix=CHECK-CUSTOM-CALL | ||
|
||
void a() { | ||
} | ||
|
||
// CHECK-CUSTOM-CALL: warning: Custom message [custom-call] | ||
// CHECK-CUSTOM-CALL-NEXT: a();{{$}} | ||
void b() { | ||
a(); | ||
} | ||
|
||
//Check let with match expression | ||
// RUN: clang-tidy %s -checks='-*, custom-*' \ | ||
// RUN: -config="{ClangQueryChecks: {%{custom-let-call-yaml}}}" \ | ||
// RUN: -- | FileCheck %s -check-prefix=CHECK-CUSTOM-LET | ||
void c() { | ||
// CHECK-CUSTOM-LET: warning: Let message [custom-let-call] | ||
// CHECK-CUSTOM-LET-NEXT: long long test_long_long = 0;{{$}} | ||
long long test_long_long_nolint = 0; //NOLINT(custom-let-call) | ||
long long test_long_long = 0; | ||
} | ||
|
||
//Check multiple checks in one config | ||
// RUN: clang-tidy %s -checks='-*, custom-*' \ | ||
// RUN: -config=%{full-config} \ | ||
// RUN: -- | FileCheck %s -check-prefixes=CHECK-CUSTOM-CALL,CHECK-CUSTOM-LET | ||
|
||
//Check multiple checks in one config but only one enabled | ||
// RUN: clang-tidy %s -checks='-*, custom-call' \ | ||
// RUN: -config=%{full-config} \ | ||
// RUN: -- | FileCheck %s -check-prefixes=CHECK-CUSTOM-CALL --implicit-check-not warning: | ||
|
||
//Check config dump | ||
// RUN: clang-tidy -dump-config -checks='-*, custom-*' \ | ||
// RUN: -config=%{full-config} \ | ||
// RUN: -- | FileCheck %s -check-prefix=CHECK-CONFIG | ||
// CHECK-CONFIG: ClangQueryChecks: | ||
// CHECK-CONFIG-DAG: custom-let-call: | ||
// CHECK-CONFIG-DAG: custom-call: |{{$[[:space:]]}} m callExpr().bind("Custom message") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing test for --verify-config and --list-checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this introduces new dependency,
how it will impact for example clangd ?
will those checks work there ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is good to compare clangtidy binary size change before and after, since clangquery is not a small binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will need to check that. It seems it might not see those checks
With the following cmake flags
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra;lldb" -DLLVM_OPTIMIZED_TABLEGEN=ON -DLLVM_USE_SPLIT_DWARF=ON -DLLVM_USE_LINKER=lld
clang-query 52M -> 52M
clang-tidy 133M -> 135M
clangd 182M -> 184M