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
Simon Fraser (smfr)
2019-07-20 11:41:01 PDT
Created attachment 374559 [details]
Patch
Comment on attachment 374559 [details] Patch Clearing flags on attachment: 374559 Committed r247688: <https://trac.webkit.org/changeset/247688> All reviewed patches have been landed. Closing bug. 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. 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. (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. |