[PATCH] Enhance clang-tidy readability-simplify-boolean-expr check to handle chained conditional assignment and chained conditional return

[Richard]

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:121
@@ +120,3 @@
+
+bool get(const OptionsView &Options, StringRef LocalName) {
+  return Options.get(LocalName, 0U) != 0U;
----------------
LegalizeAdulthood wrote:
> alexfh wrote:
> > I don't think this function makes code shorter or easier to understand. I'd remove it and use the implementation directly (maybe with implicit conversion to bool to avoid the ` == 0U` part).
> I meant to bring this up on the mailing list.  I got stuck for a while because `bool b = false; Options.get(LocalName, b);` totally failed when `LocalName` was configured to `true`.  I tried to dig into it and figure out why, apparently there is a member function template that is `enable_if`'ed for integral types and because `bool` is an integral type, it was trying to turn `true` into an integer and silently failing, leaving my boolean setting unset.  I'd rather have the setting just plain work for booleans because it's weird to turn this yes/no option into 1/0 in the configuration.  So I was trying to have a local function that did "what I wanted" from OptionView until I could figure out how to get OptionView to properly handle boolean options.
Fixed.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:211
@@ -201,5 +210,3 @@
                                                   bool Value, StringRef Id) {
-  Finder->addMatcher(ifStmt(isExpansionInMainFile(),
-                            hasThen(ReturnsBool(Value, ThenLiteralId)),
-                            hasElse(ReturnsBool(!Value))).bind(Id),
-                     this);
+  if (ChainedConditionalReturn) {
+    Finder->addMatcher(ifStmt(isExpansionInMainFile(),
----------------
LegalizeAdulthood wrote:
> alexfh wrote:
> > How about:
> > 
> >       Finder->addMatcher(ifStmt(isExpansionInMainFile(),
> >                               ChainedConditionalReturn ? anything() : unless(hasParent(ifStmt())),
> >                               hasThen(ReturnsBool(Value, ThenLiteralId)),
> >                               hasElse(ReturnsBool(!Value))).bind(Id),
> >                        this);
> > 
> > ? (if this works, of course)
> > 
> > Same below.
> I thought about that, but I didn't try it out.  I can go either way.
I tried this and it gets a compile error because `anything()` and `unless(hasParent(ifStmt()))` don't have compatible types.

http://reviews.llvm.org/D8996

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/