Re: Hiding 'new' statements - Good or Evil?

[Nathan Froyd]

On Fri, Nov 24, 2017 at 11:35 AM, Eric Rescorla  wrote:
> On Thu, Nov 23, 2017 at 4:00 PM, smaug  wrote:
>> I guess I'd prefer UniquePtr::New() over MakeUnique to be more clear about
>> the functionality.
>
> This seems like a reasonable argument in isolation, but I think it's more
> important to mirror the standard C++ mechanisms and C++-14 already defines
> std::make_unique.
>
> As a post-script, given that we now can use C++-14, can we globally replace
> the MFBT clones of C++-14 mechanisms with the standard ones?

In general, no.

std::unique_ptr is not a drop-in replacement for mozilla::UniquePtr,
for instance, and people seem OK with that--plus maintaining our own
smart pointer class opens up optimization opportunities the standard
library can only implement with difficulty[0].  In a similar fashion,
having our own mozilla::Move means we can implement checks on the use
of Move that the standard library version can't.  std::vector is not
an adequate replacement for mozilla::Vector.  And so forth.

There are specific instances where using the standard library doesn't
seem problematic: we have a bug open on replacing TypeTraits.h with
type_traits, but that has run into issues with how we wrap STL
headers, and nobody has devoted the time to figuring out how to deal
with the issues.  But each instance would have to be considered on the
merits.

-Nathan

[0] See http://lists.llvm.org/pipermail/cfe-dev/2017-November/055955.html
and the ensuing discussion.  There was discussion of standardizing
this attribute, but it's not clear to me that std::unique_ptr could
immediately take advantage of this without ABI breakage.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Hiding 'new' statements - Good or Evil?

[smaug]

On 11/24/2017 06:35 PM, Eric Rescorla wrote:

On Thu, Nov 23, 2017 at 4:00 PM, smaug  wrote:


On 11/23/2017 11:54 PM, Botond Ballo wrote:


I think it makes sense to hide a 'new' call in a Make* function when
you're writing an abstraction that handles allocation *and*
deallocation.

So MakeUnique makes sense, because UniquePtr takes ownership of the
allocated object, and will deallocate it in the destructor. MakeRefPtr
would make sense for the same reason.


I almost agree with this, but, all these Make* variants hide the
information that they are allocating,
and since allocation is slow, it is usually better to know when allocation
happens.
I guess I'd prefer UniquePtr::New() over MakeUnique to be more clear about
the functionality.



This seems like a reasonable argument in isolation, but I think it's more
important to mirror the standard C++ mechanisms and C++-14 already defines
std::make_unique.


Is it? Isn't it more important to write less error prone code and code where 
the reader
sees the possible performance issues.
Recent-ish C++ additions have brought quite a few bad concepts which we 
shouldn't use
without considering whether they actually fit into our needs.
Something being new and hip and cool doesn't mean it is actually a good thing.

(This broken record keeps reminding about the security issues and memory leaks 
auto and ranged-for have caused.)




As a post-script, given that we now can use C++-14, can we globally replace
the MFBT clones of C++-14 mechanisms with the standard ones?

-Ekr





-Olli




But in cases where a library facility is not taking ownership of the
object, and thus the user will need to write an explicit 'delete', it
makes sense to require that the user also write an explicit 'new', for
symmetry.

NotNull is a bit of a weird case because it can wrap either a raw
pointer or a smart pointer, so it doesn't clearly fit into either
category. Perhaps it would make sense for MakeNotNull to only be
usable with smart pointers?

Cheers,
Botond



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform