[webkit-reviews] review denied: [Bug 132348] NeverDestroyed objects should be constructable through std::initializer_list : [Attachment 230463] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Zan Dobersek <zandobersek at gmail.com>'s
request for review:
Bug 132348: NeverDestroyed objects should be constructable through
std::initializer_list
https://bugs.webkit.org/show_bug.cgi?id=132348

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

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


> Source/WebCore/editing/EditingStyle.cpp:754
> +    static NeverDestroyed<Vector<std::unique_ptr<HTMLElementEquivalent>>>
equivalents = {
> +	   std::make_unique<HTMLElementEquivalent>(CSSPropertyFontWeight,
CSSValueBold, HTMLNames::bTag),
> +	   std::make_unique<HTMLElementEquivalent>(CSSPropertyFontWeight,
CSSValueBold, HTMLNames::strongTag),
> +	   std::make_unique<HTMLElementEquivalent>(CSSPropertyVerticalAlign,
CSSValueSub, HTMLNames::subTag),
> +	   std::make_unique<HTMLElementEquivalent>(CSSPropertyVerticalAlign,
CSSValueSuper, HTMLNames::supTag),
> +	   std::make_unique<HTMLElementEquivalent>(CSSPropertyFontStyle,
CSSValueItalic, HTMLNames::iTag),
> +	   std::make_unique<HTMLElementEquivalent>(CSSPropertyFontStyle,
CSSValueItalic, HTMLNames::emTag),
> +	  
std::unique_ptr<HTMLElementEquivalent>(std::make_unique<HTMLTextDecorationEquiv
alent>(CSSValueUnderline, HTMLNames::uTag)),
> +	  
std::unique_ptr<HTMLElementEquivalent>(std::make_unique<HTMLTextDecorationEquiv
alent>(CSSValueLineThrough, HTMLNames::sTag)),
> +	  
std::unique_ptr<HTMLElementEquivalent>(std::make_unique<HTMLTextDecorationEquiv
alent>(CSSValueLineThrough, HTMLNames::strikeTag)),
> +    };

Anders has suggested that we not make initializer lists support move
semantics--I think he’s right and so we should not make this change here.

The explicit cast to a different type of unique_ptr is also quite ugly.

Maybe there’s some other improvement we can make here, but we shouldn’t do
exactly this.

> Source/WebCore/editing/EditingStyle.cpp:787
> +	   std::make_unique<HTMLAttributeEquivalent>(CSSPropertyColor,
HTMLNames::fontTag, HTMLNames::colorAttr),
> +	   std::make_unique<HTMLAttributeEquivalent>(CSSPropertyFontFamily,
HTMLNames::fontTag, HTMLNames::faceAttr),
> +	  
std::unique_ptr<HTMLAttributeEquivalent>(std::make_unique<HTMLFontSizeEquivalen
t>()),
> +	  
std::unique_ptr<HTMLAttributeEquivalent>(std::make_unique<HTMLAttributeEquivale
nt>(CSSPropertyDirection, HTMLNames::dirAttr)),
> +	  
std::unique_ptr<HTMLAttributeEquivalent>(std::make_unique<HTMLAttributeEquivale
nt>(CSSPropertyUnicodeBidi, HTMLNames::dirAttr)),

Ditto.