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

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 12 08:11:24 PST 2015


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)


>
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151112/ef00250d/attachment.html>


More information about the cfe-commits mailing list