RFR: 8234779: Provide idiom for declaring classes noncopyable
per.liden at oracle.com
Thu Nov 28 08:50:11 UTC 2019
On 11/27/19 10:49 PM, Kim Barrett wrote:
>> On Nov 27, 2019, at 3:43 AM, Per Liden <per.liden at oracle.com> wrote:
>> Please don't add this :( I don't think this adds any value, it add another ugly macros I know I will never want to use. I'd much prefer to read real C++ instead of some macro that hides what's going on.
> Not being explicit about copy functions or noncopyability (e.g. not
> following the Rule of 3) can and has resulted in bugs. C++ will
> silently create the used functions with default definitions that
> aren't at all what one wants in some cases.
> The Rule of 3 makes it easer to read and understand code, because
> certain classes of easily overlooked errors are prevented by the
> compiler and simply cannot happen by design. That's why it's a "rule"
> in the wider community, even though not so much in HotSpot code, to
> our detriment in my opinion.
> The C++03 idiom of private declared but not defined copy ctor and
> assignment operator is, so far as I know, the best mechanism available
> for making a class noncopyable. All other approaches I know of have
> unpleasant side effects.
No objections here.
> That idiom is rather wordy and indirect though. In particular, it
> is generally accompanied by comments indicating that this is to make
> the class noncopyable, or that the declared functions are not defined
> (not always with a reason, so that needs to be inferred). Failure to
> provide such comments means the reader may need to check for a
> definition in order to determine whether that idiom is being used, or
> whether the definitions are just not inline.
> The proposed macro significantly reduces that wordiness. Far more
> importantly, it makes the intent entirely self-evident; there's no
> need for any explanatory comments.
My objection is that you are effectively moving us _away_ from a well
known C++idiom, since people tend to read code before it goes through
the pre-processor. Once we have C++11 support we can easily switch over
to using "= delete", and anything that was previously ambiguous or
needed a comment will become clear, and our code would stay idiomatic.
> He C++11 idiom is slightly different, in that deleted definitions
> should be used rather than leaving the operations undefined. That's
> easily accomodated with this macro; a couple of small changes to the
> macro and all uses are done. (There is a benefit to making the
> deleted definitions public with C++11, probably getting a better error
> message, but that's chrome and can be improved lazily as code gets
Adding "= delete" is good, but it will be no more work than it was to
create this patch, so that can't be an argument in favor of this patch.
More information about the hotspot-dev