[webkit-reviews] review granted: [Bug 217855] Fix -Wdeprecated-copy warnings in WTF and JavaScriptCore : [Attachment 411628] Patch v1

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 217855: Fix -Wdeprecated-copy warnings in WTF and JavaScriptCore
https://bugs.webkit.org/show_bug.cgi?id=217855

Attachment 411628: Patch v1

https://bugs.webkit.org/attachment.cgi?id=411628&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 411628
  --> https://bugs.webkit.org/attachment.cgi?id=411628
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=411628&action=review

> Source/JavaScriptCore/assembler/ARM64Assembler.h:375
> +	       data.copyTypes.content[0] = other.data.copyTypes.content[0];
> +	       data.copyTypes.content[1] = other.data.copyTypes.content[1];
> +	       data.copyTypes.content[2] = other.data.copyTypes.content[2];

Could we do this with construction syntax? Maybe we can’t because of the union.

> Source/JavaScriptCore/assembler/ARM64Assembler.h:377
> +	   ~LinkRecord() = default;

Why add this?

> Source/WTF/wtf/Identified.h:42
> +    IdentifiedBase(const IdentifiedBase&) = default;

This change continues to let the compiler generate a public copy assignment
operator, despite the fact that we got out of the way to make the copy
constructor protected. It’s more logical that they are either both public or
both protected. I suggest we either make both protected by adding the
assignment operator explicitly, or both public, by deleting this explicit copy
constructor.