ymandel created this revision.
ymandel added a reviewer: gribozavr2.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.
ymandel requested review of this revision.
Adds support for setting the `Rule` field. In the process, refactors the code
that accesses that field and adds a constructor that doesn't require a rule
argument.
This feature is needed by checks that must set the rule *after* the check class
is constructed. For example, any check that maintains state to be accessed from
the rule needs this support. Since the object's fields are not initialized when
the superclass constructor is called, they can't be (safely) captured by a rule
passed to the existing constructor. This patch allows constructing the check
superclass fully before setting the rule.
As a driveby fix, removed the "optional" from the rule, since rules are just a
set of cases, so empty rules are evident.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D91544
Files:
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
@@ -38,6 +38,8 @@
// includes.
class TransformerClangTidyCheck : public ClangTidyCheck {
public:
+ TransformerClangTidyCheck(StringRef Name, ClangTidyContext *Context);
+
// \p MakeRule generates the rewrite rule to be used by the check, based on
// the given language and clang-tidy options. It can return \c None to handle
// cases where the options disable the check.
@@ -68,8 +70,11 @@
/// the overridden method.
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ /// Set the rule that this check implements.
+ void setRule(transformer::RewriteRule R);
+
private:
- Optional<transformer::RewriteRule> Rule;
+ transformer::RewriteRule Rule;
IncludeInserter Inserter;
};
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -21,6 +21,18 @@
}
#endif
+static void verifyRule(const RewriteRule &Rule) {
+ assert(llvm::all_of(Rule.Cases, hasExplanation) &&
+ "clang-tidy checks must have an explanation by default;"
+ " explicitly provide an empty explanation if none is desired");
+}
+
+TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ Inserter(
+ Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {}
+
// This constructor cannot dispatch to the simpler one (below), because, in
// order to get meaningful results from `getLangOpts` and `Options`, we need the
// `ClangTidyCheck()` constructor to have been called. If we were to dispatch,
@@ -31,24 +43,21 @@
const OptionsView &)>
MakeRule,
StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context), Rule(MakeRule(getLangOpts(), Options)),
- Inserter(
- Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {
- if (Rule)
- assert(llvm::all_of(Rule->Cases, hasExplanation) &&
- "clang-tidy checks must have an explanation by default;"
- " explicitly provide an empty explanation if none is desired");
+ : TransformerClangTidyCheck(Name, Context) {
+ if (Optional<RewriteRule> R = MakeRule(getLangOpts(), Options))
+ setRule(std::move(*R));
}
TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
StringRef Name,
ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context), Rule(std::move(R)),
- Inserter(
- Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {
- assert(llvm::all_of(Rule->Cases, hasExplanation) &&
- "clang-tidy checks must have an explanation by default;"
- " explicitly provide an empty explanation if none is desired");
+ : TransformerClangTidyCheck(Name, Context) {
+ setRule(std::move(R));
+}
+
+void TransformerClangTidyCheck::setRule(transformer::RewriteRule R) {
+ verifyRule(R);
+ Rule = std::move(R);
}
void TransformerClangTidyCheck::registerPPCallbacks(
@@ -58,8 +67,8 @@
void TransformerClangTidyCheck::registerMatchers(
ast_matchers::MatchFinder *Finder) {
- if (Rule)
- for (auto &Matcher : transformer::detail::buildMatchers(*Rule))
+ if (!Rule.Cases.empty())
+ for (auto &Matcher : transformer::detail::buildMatchers(Rule))
Finder->addDynamicMatcher(Matcher, this);
}
@@ -68,8 +77,7 @@
if (Result.Context->getDiagnostics().hasErrorOccurred())
return;
- assert(Rule && "check() should not fire if Rule is None");
- RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, *Rule);
+ RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, Rule);
Expected<SmallVector<transformer::Edit, 1>> Edits = Case.Edits(Result);
if (!Edits) {
llvm::errs() << "Rewrite failed: " << llvm::toString(Edits.takeError())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
