Bug 199981

Summary: Make some constructors explicit
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: WebKit Misc.Assignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, dbates, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=200006
Attachments:
Description Flags
Patch none

Description Simon Fraser (smfr) 2019-07-20 11:41:01 PDT
Make some constructors explicit
Comment 1 Simon Fraser (smfr) 2019-07-20 11:42:42 PDT
Created attachment 374559 [details]
Patch
Comment 2 WebKit Commit Bot 2019-07-22 11:05:23 PDT
Comment on attachment 374559 [details]
Patch

Clearing flags on attachment: 374559

Committed r247688: <https://trac.webkit.org/changeset/247688>
Comment 3 WebKit Commit Bot 2019-07-22 11:05:24 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Radar WebKit Bug Importer 2019-07-22 11:06:20 PDT
<rdar://problem/53406381>
Comment 5 Darin Adler 2019-07-22 11:30:04 PDT
Comment on attachment 374559 [details]
Patch

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

Surprised that we are adding explicit to constructors that take multiple arguments since I don’t think it has any effect in that case (without default values for the arguments).

> Source/WebCore/Modules/webdatabase/ChangeVersionData.h:34
> +    explicit ChangeVersionData(String oldVersion, String newVersion)

Makes no sense to use explicit for a constructor with multiple arguments. Maybe does no harm but I believe it has no effect.

> Source/WebCore/Modules/websockets/WebSocketExtensionParser.h:41
> +    explicit WebSocketExtensionParser(const char* start, const char* end)

Ditto.

> Source/WebCore/PAL/pal/system/cocoa/SleepDisablerCocoa.h:36
> +    explicit SleepDisablerCocoa(const char*, Type);

Ditto.

> Source/WebCore/css/parser/SizesAttributeParser.h:44
> +    explicit SizesAttributeParser(const String&, const Document&);

Ditto.

> Source/WebCore/dom/SpaceSplitString.cpp:106
> +    explicit TokenIsEqualToCStringTokenProcessor(const char* referenceString, unsigned referenceStringLength)

Ditto.

> Source/WebCore/platform/audio/DynamicsCompressorKernel.h:40
> +    explicit DynamicsCompressorKernel(float sampleRate, unsigned numberOfChannels);

Ditto.

> Source/WebCore/platform/audio/MultiChannelResampler.h:43
> +    explicit MultiChannelResampler(double scaleFactor, unsigned numberOfChannels);

Ditto.

> Source/WebCore/platform/audio/SincResampler.cpp:149
> +    explicit BufferSourceProvider(const float* source, size_t numberOfSourceFrames)

Ditto.

> Source/WebCore/platform/audio/ios/AudioFileReaderIOS.h:47
> +    explicit AudioFileReader(const void* data, size_t dataSize);

Ditto.

> Source/WebCore/platform/audio/mac/AudioFileReaderMac.h:45
> +    explicit AudioFileReader(const void* data, size_t dataSize);

Ditto.

> Source/WebCore/platform/cf/KeyedDecoderCF.h:37
> +    explicit KeyedDecoderCF(const uint8_t* data, size_t);

Ditto.

> Source/WebCore/platform/graphics/WidthIterator.cpp:68
> +    explicit OriginalAdvancesForCharacterTreatedAsSpace(bool isSpace, float advanceBefore, float advanceAt)

Ditto.

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.h:38
> +    explicit SynthesisPair(bool needsSyntheticBold, bool needsSyntheticOblique)

Ditto.

> Source/WebCore/platform/text/TextCodecICU.h:38
> +    explicit TextCodecICU(const char* encoding, const char* canonicalConverterName);

Ditto.

> Source/WebCore/rendering/RenderTableSection.h:45
> +    explicit CellSpan(unsigned start, unsigned end)

Ditto.

> Source/WebCore/testing/MockLibWebRTCPeerConnection.h:115
> +    explicit MockLibWebRTCIceCandidate(const char* sdp, const char* sdpMid)

Ditto.
Comment 6 Daniel Bates 2019-07-22 17:01:33 PDT
It has an effect: No unnamed aggregate initialization. <--- eh, I can go either way here, I usually pick the style that's clearest for the call site.
Comment 7 Darin Adler 2019-07-22 18:14:52 PDT
(In reply to Daniel Bates from comment #6)
> It has an effect: No unnamed aggregate initialization.

Good to know. Given that new information, I suggest we use explicit most of the time on almost all constructors.