[webkit-reviews] review denied: [Bug 124866] VS2010 doesn't like std::make_unique : [Attachment 219386] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 17 09:15:26 PST 2013


Darin Adler <darin at apple.com> has denied Alex Christensen
<alex.christensen at flexsim.com>'s request for review:
Bug 124866: VS2010 doesn't like std::make_unique
https://bugs.webkit.org/show_bug.cgi?id=124866

Attachment 219386: Patch
https://bugs.webkit.org/attachment.cgi?id=219386&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=219386&action=review


> Source/WebCore/platform/graphics/TiledBackingStore.h:44
> -    TiledBackingStore(TiledBackingStoreClient*,
std::unique_ptr<TiledBackingStoreBackend> =
std::make_unique<TiledBackingStoreBackend>());
> +    TiledBackingStore(TiledBackingStoreClient*,
std::unique_ptr<TiledBackingStoreBackend> =
std::unique_ptr<TiledBackingStoreBackend>(new TiledBackingStoreBackend()));

Please don’t do this. Is there some other solution? The reason we use
make_unique is so that we can search the entire project for "new" to find
mistakes. We really don’t want to do this just to accommodate a broken
compiler.

I suggest we use overloading instead:

    TiledBackingStore(TiledBackingStoreClient*);
    TiledBackingStore(TiledBackingStoreClient*,
std::unique_ptr<TiledBackingStoreBackend>);

Then the make_unique can go into the TiledBackingStore.cpp file. I’m hoping we
can get it to compile there.

By the way, this overloading also makes it clear that the
single-non-default-argument constructor needs “explicit” added to it. Please do
that.


More information about the webkit-reviews mailing list