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

Reply via email to