lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lmi] C11n: should we use "= default"?


From: Vadim Zeitlin
Subject: [lmi] C11n: should we use "= default"?
Date: Tue, 29 Nov 2016 01:26:00 +0100

 Hello,

 I've created a tentative pull request which replaces the empty bodies of
(almost) all ctors and dtors with the C++11 "= default" declaration at

        https://github.com/vadz/lmi/pull/47

I'm saying that it's tentative because I'm not really sure if you're
willing to entertain the idea of applying this and I don't think it's as
important as the other C11n patches, such as the (trivial but useful)
already applied "nullptr" one or the (also trivial but even more useful)
"override" one at https://github.com/vadz/lmi/pull/46 -- this one is not
completely trivial and, looking at the result, I'm not sure if it really
helps making the code more readable, so I'd like to have your opinion.

 From my point of view, while it's true that this change formally
simplifies the code and cuts down on the number of lines (260 fewer after
this change), it also just raises more questions about why are we declaring
these default dtors in the first place. I've looked at a few of them and
some are really needed, but not all of them are and I don't really know
whether I should remove the dtors that don't seem to be needed at all to
make things more logical, but even more difficult for you to review, or
leave them like this.

 For now I'm probably not going to do anything and rather replace the use
of lmi::uncopyable with the deleted copy ctors/assignment operators as this
would, IMO, be a more clear gain. But please let me know if you'd like me
to do anything else/more about the default ctors/dtors or even if you're,
by chance, happy with the patch in its current state, i.e. without any non
trivial changes (with the exception of one mentioned in the commit
message).

 In any case, please notice that you should _not_ apply this patch
currently as it would result in (manageably small but, still, why have
them) conflicts with the much more important "override" PR #46. If you do
want to apply at all, please let me know and I'll rebase it on the latest
master later, when you're ready to apply it.

 Thanks in advance!
VZ


reply via email to

[Prev in Thread] Current Thread [Next in Thread]