[PATCH] Refactor: Simplify boolean expressions in lib/AST

[Daniel Jasper]

I am actually not sure how clang-tidy should handle cases like this (where
two checks would fix the same code in different ways) and it's probably
something we should figure out in the long run.

In this case, the else after return (yes, we do have a check for that
although it doesn't yet offer automatic fixing) should take precedence as
there is an explicit rule in the coding standards.

Of course, that still leaves the question of whether clang-tidy should also
simplify:

  if (a)
    return true;
  return false;

to

  return a;

On Tue, Mar 24, 2015 at 7:41 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Mon, Mar 23, 2015 at 11:30 PM, Daniel Jasper <djasper at google.com>
> wrote:
>
>> I don't feel strongly about this, and I can see some of your reasoning.
>> However, an "if (a) return true; else return false;" is very suspect to me
>> and I think "return a;" is more readable, independent of whether it is at
>> the end of a chain or not.
>>
>> However, there is an underlying issue here. The existing code is
>> violating LLVM's coding standard of "don't use else after return" (
>> http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return).
>> So in these chains, all of the "else"s should be removed.
>>
>
> & that incidentally (sounds like more by accident than design) suppresses
> the warning. Though, arguably, this clang-tidy check perhaps shouldn't be
> passing judgment on that particular case and we should have an
> else-after-return check (we probably already do) for that explicitly.
>
> That said, I don't much mind this check happening to provide
> else-after-return catching in this way. (which is to say, in a round about
> way, I agree with you Daniel) Maybe it's just a case of two different style
> checkers happening to catch the same code for different reasons, and the
> same potential code change addresses both of them.
>
>
>>
>> On Tue, Mar 24, 2015 at 7:17 AM, Richard <legalize at xmission.com> wrote:
>>
>>> Thanks for your ideas, John, I'm going to incorporate this into the
>>> `clang-tidy` check.
>>>
>>>
>>> http://reviews.llvm.org/D8532
>>>
>>> EMAIL PREFERENCES
>>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150324/0ea2b6f2/attachment.html>