On Thu, Nov 12, 2015 at 11:11 AM, David Blaikie <dblai...@gmail.com> wrote: > > > On Thu, Nov 12, 2015 at 6:36 AM, Aaron Ballman via cfe-commits > <cfe-commits@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits