Summary: | Clients of optional should use has_value instead of relying on hasValue macro | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||
Component: | WebKit Misc. | Assignee: | Darin Adler <darin> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | alecflett, beidson, benjamin, berto, calvaris, cdumez, cgarcia, changseok, cmarcelo, dino, eric.carlson, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, gustavo, gyuyoung.kim, hi, jamesr, japhet, jer.noble, jfernandez, jiewen_tan, joepeck, jsbell, kangil.han, keith_miller, kondapallykalyan, luiz, macpherson, mark.lam, menard, mifenton, msaboff, pdr, philipj, pnormand, rego, saam, sabouhallawa, sam, schenney, sergio, svillar, tonikitoo, tzagallo, vjaquez, webkit-bug-importer | ||||||
Priority: | P3 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Darin Adler
2021-05-28 14:34:30 PDT
Created attachment 430059 [details]
Patch
Couldn’t do this with a global replace because hasValue is a common name used in classes other than WTF::Optional. First version will probably fail EWS if I missed things on non-Mac platforms. Created attachment 430077 [details]
Patch
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 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(). Committed r278244 (238281@main): <https://commits.webkit.org/238281@main> |