omtcyfz added inline comments. ================ Comment at: clang-tidy/modernize/UseBoolLiteralsCheck.cpp:51-52 @@ -34,4 +50,4 @@ void UseBoolLiteralsCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Literal = Result.Nodes.getNodeAs<IntegerLiteral>("literal"); - const auto *Cast = Result.Nodes.getNodeAs<Expr>("cast"); - bool LiteralBooleanValue = Literal->getValue().getBoolValue(); + for (const auto &BindingName : + {"literal", "trueBranchLiteral", "falseBranchLiteral"}) { + const auto *Literal = Result.Nodes.getNodeAs<IntegerLiteral>(BindingName); ---------------- aaron.ballman wrote: > omtcyfz wrote: > > aaron.ballman wrote: > > > omtcyfz wrote: > > > > aaron.ballman wrote: > > > > > Any reason not to name the bind "literal" in all three cases? That > > > > > eliminates the need for the loop entirely, since `check()` will > > > > > trigger for each instance of a match. > > > > It doesn't make sense to try binding both `TrueExpression` and > > > > `FalseExpression` literals to a single value. > > > Why? In all three cases, you don't care what matched, just that *some* > > > case is matched. None of the logic in `check()` relies on which part of > > > the expression is matched. > > Well, in case of second matcher I may have **two** literals matched upon > > triggering. I don't understand how I could possibly get **two** literals > > bound to **one** value after **one** matcher got triggered. > > > > Am I missing something? > One matcher isn't what's getting triggered then, is it? I could be wrong on > this point, but I thought that in that case, `check()` would be called twice, > once for each literal. Is that not the case? From what I understand about how matchers work, it is not. Plus, I checked (just named everything `"literal"` and removed `for (const auto ...` just to double check in case I was wrong.
https://reviews.llvm.org/D23243 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits