[PATCH] Refactor: Simplify boolean expressions in lib/AST
David Blaikie
dblaikie at gmail.com
Mon Mar 23 23:41:49 PDT 2015
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/20150323/74f45ea1/attachment.html>
More information about the cfe-commits
mailing list