[PATCH] D14560: [Clang] Fix Clang-tidy modernize-use-auto in some files in lib/AST; other minor cleanups.

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 12 12:18:12 PST 2015


On Thu, Nov 12, 2015 at 11:11 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Thu, Nov 12, 2015 at 6:36 AM, Aaron Ballman via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>>
>> aaron.ballman added inline comments.
>>
>> ================
>> Comment at: lib/AST/ASTContext.cpp:7930
>> @@ -7931,3 +7929,3 @@
>>
>> -ASTMutationListener::~ASTMutationListener() { }
>> +ASTMutationListener::~ASTMutationListener() = default;
>>
>> ----------------
>> Eugene.Zelenko wrote:
>> > aaron.ballman wrote:
>> > > This is... interesting. Explicitly defaulting an out-of-line function
>> > > definition is not something I've ever encountered before. What benefit does
>> > > this provide?
>> > It's explicitly tells that destructor has default implementation.
>> I'm not certain this is an improvement. Using =default in the declaration
>> is an improvement because it tells the compiler up front "this has the
>> default implementation" and the compiler can benefit from that information
>> in other ways. When it's on an out-of-line definition, it does say "this has
>> the default implementation", but it doesn't give the compiler any benefit
>> over {}, so the only benefit is up to the reader of the code. I think
>> someone can read {} to easily tell it is the default behavior, it is
>> considerably shorter, and it doesn't cause anyone's eyes to trip over it
>> wondering about the construct.
>
>
> If we're talking coding style, I'll vote marginally in favor of "= default"
> even in out of line definitions. But yeah, I don't think it necessarily adds
> much either *shrug* I probably wouldn't hold up a code review on it either
> way myself. (this is not a directive, just my own commentary)

The good news is, I'm only marginally against "= default"! :-D I can
go either way, but many of the changes in the patch are not ones that
I think we wish to accept. We should address those first, and then we
can determine what road to take for out-of-line defaulted functions.

~Aaron

>
>>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> http://reviews.llvm.org/D14560
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>


More information about the cfe-commits mailing list