[PATCH] D56323: [clang-tidy] Handle member variables in readability-simplify-boolean-expr

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 5 14:09:03 PST 2019


JonasToth added inline comments.


================
Comment at: test/clang-tidy/readability-simplify-bool-expr-members.cpp:160
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}} in ternary expression result
+  // CHECK-FIXES: {{^  m_b2 = i <= 20;$}}
+
----------------
LegalizeAdulthood wrote:
> JonasToth wrote:
> > LegalizeAdulthood wrote:
> > > JonasToth wrote:
> > > > For this case the logical inversion is simple an obviously `<=`, but what if the condition is more complex?
> > > > 
> > > > I would personally prefer `!(i < 20)` instead + complex logical expressions as additional test. They would be interesting for the `if` cases, too.
> > > If you have complex conditions, you can run the check repeatedly until you reach a fixed point.
> > > 
> > > Using !(x) instead of the reverse operator could be added as an enhancement, but it is not covered by this patch.
> > But the current version is the enhancement.
> > The complex condition, like `i < 20 && i > 0` will be the common case, so the inversion (with parens) should always be used (`!(i < 20 && i > 0)`) and specializations can be done later.
> No, the current version is a bug fix.  The simplification of boolean expressions was already there, but it didn't handle member variables properly.
> 
> You're asking for an additional enhancement about the way boolean expressions are simplified.  That's fine for an additional enhancement of the check, but it is not the point of this patch.
> 
> The complex expression you're discussing in your comment is not changed by this change, nor is it changed by the existing check.
I see, then I did misunderstand these changes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56323/new/

https://reviews.llvm.org/D56323





More information about the cfe-commits mailing list