looking now... On Fri, May 24, 2019 at 12:49 PM Ilya Biryukov <ibiryu...@google.com> wrote:
> 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
>
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
