This seems to produce warnings about unused variables:
…/llvm/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:20:20:
warning: unused variable 'Case' [-Wunused-variable]
for (const auto &Case : Rule.Cases) {
Could you take a look?
On Fri, May 24, 2019 at 6:29 PM Yitzhak Mandelbaum via Phabricator <
revi...@reviews.llvm.org> wrote:
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL361647: [clang-tidy] In TransformerClangTidyCheck,
> require Explanation field. (authored by ymandel, committed by ).
> Herald added a project: LLVM.
> Herald added a subscriber: llvm-commits.
>
> Changed prior to commit:
> https://reviews.llvm.org/D62340?vs=201256&id=201272#toc
>
> Repository:
> rL LLVM
>
> CHANGES SINCE LAST ACTION
> https://reviews.llvm.org/D62340/new/
>
> https://reviews.llvm.org/D62340
>
> Files:
> clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp
> clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.h
>
> clang-tools-extra/trunk/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
>
>
> Index: clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.h
> ===================================================================
> --- clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.h
> +++ clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.h
> @@ -31,10 +31,14 @@
> // };
> class TransformerClangTidyCheck : public ClangTidyCheck {
> public:
> + // All cases in \p R must have a non-null \c Explanation, even though \c
> + // Explanation is optional for RewriteRule in general. Because the
> primary
> + // purpose of clang-tidy checks is to provide users with diagnostics, we
> + // assume that a missing explanation is a bug. If no explanation is
> desired,
> + // indicate that explicitly (for example, by passing `text("no
> explanation")`
> + // to `makeRule` as the `Explanation` argument).
> TransformerClangTidyCheck(tooling::RewriteRule R, StringRef Name,
> - ClangTidyContext *Context)
> - : ClangTidyCheck(Name, Context), Rule(std::move(R)) {}
> -
> + ClangTidyContext *Context);
> void registerMatchers(ast_matchers::MatchFinder *Finder) final;
> void check(const ast_matchers::MatchFinder::MatchResult &Result) final;
>
> Index:
> clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp
> ===================================================================
> --- clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp
> +++ clang-tools-extra/trunk/clang-tidy/utils/TransformerClangTidyCheck.cpp
> @@ -13,6 +13,17 @@
> namespace utils {
> using tooling::RewriteRule;
>
> +TransformerClangTidyCheck::TransformerClangTidyCheck(tooling::RewriteRule
> R,
> + StringRef Name,
> + ClangTidyContext
> *Context)
> + : ClangTidyCheck(Name, Context), Rule(std::move(R)) {
> + for (const auto &Case : Rule.Cases) {
> + assert(Case.Explanation != nullptr &&
> + "clang-tidy checks must have an explanation by default;"
> + " explicitly provide an empty explanation if none is desired");
> + }
> +}
> +
> void TransformerClangTidyCheck::registerMatchers(
> ast_matchers::MatchFinder *Finder) {
> Finder->addDynamicMatcher(tooling::detail::buildMatcher(Rule), this);
> @@ -44,15 +55,13 @@
> if (Transformations->empty())
> return;
>
> - StringRef Message = "no explanation";
> - if (Case.Explanation) {
> - if (Expected<std::string> E = Case.Explanation(Result))
> - Message = *E;
> - else
> - llvm::errs() << "Error in explanation: " <<
> llvm::toString(E.takeError())
> - << "\n";
> + Expected<std::string> Explanation = Case.Explanation(Result);
> + if (!Explanation) {
> + llvm::errs() << "Error in explanation: "
> + << llvm::toString(Explanation.takeError()) << "\n";
> + return;
> }
> - DiagnosticBuilder Diag = diag(RootLoc, Message);
> + DiagnosticBuilder Diag = diag(RootLoc, *Explanation);
> for (const auto &T : *Transformations) {
> Diag << FixItHint::CreateReplacement(T.Range, T.Replacement);
> }
> Index:
> clang-tools-extra/trunk/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
> ===================================================================
> ---
> clang-tools-extra/trunk/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
> +++
> clang-tools-extra/trunk/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
> @@ -26,15 +26,18 @@
> using tooling::change;
> using tooling::node;
> using tooling::statement;
> + using tooling::text;
> using tooling::stencil::cat;
>
> StringRef C = "C", T = "T", E = "E";
> - return tooling::makeRule(ifStmt(hasCondition(expr().bind(C)),
> - hasThen(stmt().bind(T)),
> - hasElse(stmt().bind(E))),
> - change(statement(RewriteRule::RootID),
> - cat("if(!(", node(C), ")) ",
> statement(E),
> - " else ", statement(T))));
> + RewriteRule Rule = tooling::makeRule(
> + ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
> + hasElse(stmt().bind(E))),
> + change(
> + statement(RewriteRule::RootID),
> + cat("if(!(", node(C), ")) ", statement(E), " else ",
> statement(T))),
> + text("negate condition and reverse `then` and `else` branches"));
> + return Rule;
> }
>
> class IfInverterCheck : public TransformerClangTidyCheck {
>
>
>
--
Regards,
Ilya Biryukov
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
