Bug 226395 – Clients of optional should use has_value instead of relying on hasValue macro

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Darin Adler]

Clients of optional should use has_value instead of relying on hasValue macro

[Darin Adler]

Created attachment 430059 [details]
Patch

[Darin Adler]

Couldn’t do this with a global replace because hasValue is a common name used in classes other than WTF::Optional.

[Darin Adler]

First version will probably fail EWS if I missed things on non-Mac platforms.

[Darin Adler]

Created attachment 430077 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Chris Dumez]

Comment on attachment 430077 [details]
Patch

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

r=me

> Source/WebCore/css/FontFace.cpp:273
> +    if (!families.has_value())

Don't really need .has_value() here.

> Source/WebCore/css/FontFace.cpp:298
> +    if (!styleWrapped.has_value())

ditto.

> Source/WebCore/css/FontFace.cpp:331
> +    if (!weightWrapped.has_value())

ditto

> Source/WebCore/css/FontFace.cpp:354
> +    if (!stretchWrapped.has_value())

ditto.

> Source/WebCore/css/FontFace.cpp:377
> +    if (!rangesWrapped.has_value())

ditto.

> Source/WebCore/css/FontFace.cpp:392
> +    if (!featureSettingsWrapped.has_value())

ditto.

> Source/WebCore/css/FontFace.cpp:407
> +    if (!loadingBehaviorWrapped.has_value())

ditto.

> Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp:116
> +    if (originalOffsetForDirectionalSnapping.has_value()) {

ditto.

> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:400
> +        m_presentationSize = getVideoResolutionFromCaps(m_demuxerSrcPadCaps.get()).value_or(WebCore::FloatSize());

probably don't need the WebCore:: in WebCore.

> Source/WebCore/svg/SVGSVGElement.cpp:317
>          transform.setA(matrixInit.a.value());

*matrixInit.a ?

> Source/WebCore/svg/SVGSVGElement.cpp:319
>          transform.setB(matrixInit.b.value());

ditto.

> Source/WebCore/svg/SVGSVGElement.cpp:321
>          transform.setC(matrixInit.c.value());

ditto.

> Source/WebCore/svg/SVGSVGElement.cpp:323
>          transform.setD(matrixInit.d.value());

ditto.

> Source/WebCore/svg/SVGSVGElement.cpp:325
>          transform.setE(matrixInit.e.value());

ditto.

> Source/WebCore/svg/SVGSVGElement.cpp:327
>          transform.setF(matrixInit.f.value());

ditto.

> Source/WebCore/svg/SVGTransform.h:81
>              transform.setA(matrixInit.a.value());

ditto in this function.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:474
> +    if (!orientation.has_value())

I don't think the .has_value() helps readability here.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:479
> +    if (!widthVariant.has_value())

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:484
> +    if (!textRenderingMode.has_value())

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:489
> +    if (!size.has_value())

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:494
> +    if (!syntheticBold.has_value())

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:499
> +    if (!syntheticOblique.has_value())

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:509
> +    if (!includesCreationData.has_value())

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:515
> +        if (!fontFaceData.has_value())

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:520
> +        if (!itemInCollection.has_value())

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:532
> +        auto creationData = WebCore::FontPlatformData::CreationData { fontFaceData.value(), itemInCollection.value() };

I would prefer *fontFaceData instead of fontFaceData.value(). It is more concise.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:533
> +        return WebCore::FontPlatformData(ctFont.get(), size.value(), syntheticBold.value(), syntheticOblique.value(), orientation.value(), widthVariant.value(), textRenderingMode.value(), &creationData);

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:538
> +    if (!referenceURL.has_value())

.has_value() is not really useful here.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:543
> +    if (!postScriptName.has_value())

ditto.

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:552
> +    return WebCore::FontPlatformData(ctFont.get(), size.value(), syntheticBold.value(), syntheticOblique.value(), orientation.value(), widthVariant.value(), textRenderingMode.value());

Could use * instead of .value().

[Darin Adler]

Committed r278244 (238281@main): <https://commits.webkit.org/238281@main>

[Radar WebKit Bug Importer]

<rdar://problem/78653580>