Bug 183357 - [WTF] Set canInitializeWithMemset = true if T is an integral type
Summary: [WTF] Set canInitializeWithMemset = true if T is an integral type
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-03-05 23:55 PST by Yusuke Suzuki
Modified: 2018-03-08 09:29 PST (History)
9 users (show)

See Also:


Attachments
Patch (7.09 KB, patch)
2018-03-06 00:15 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews206 for win-future (12.01 MB, application/zip)
2018-03-06 02:40 PST, EWS Watchlist
no flags Details
Patch (7.09 KB, patch)
2018-03-06 08:25 PST, Yusuke Suzuki
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2018-03-05 23:55:18 PST
[WTF] Set canInitializeWithMemset = true if T is an integral type
Comment 1 Yusuke Suzuki 2018-03-06 00:15:02 PST
Created attachment 335071 [details]
Patch
Comment 2 EWS Watchlist 2018-03-06 02:40:22 PST
Comment on attachment 335071 [details]
Patch

Attachment 335071 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6822587

New failing tests:
http/tests/preload/download_resources.html
Comment 3 EWS Watchlist 2018-03-06 02:40:33 PST
Created attachment 335079 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 4 Yusuke Suzuki 2018-03-06 08:25:51 PST
Created attachment 335098 [details]
Patch
Comment 5 Darin Adler 2018-03-07 20:07:57 PST
Comment on attachment 335098 [details]
Patch

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

> Source/WTF/wtf/Vector.h:71
> +    static void initializeIfNecessary(T*, T*) { }

I am not sure what "if necessary" means here. I think a C++ expert would call this initializeUnlessConstructorIsTrivial or initializeIfConstructorIsNonTrivial.

> Source/WTF/wtf/VectorTraits.h:52
> +        static const bool canInitializeWithMemset = std::is_integral<T>::value;

Why can’t we just set this to true? Are there types that return true for is_pod and can’t be initialized with memset?
Comment 6 Yusuke Suzuki 2018-03-07 22:38:29 PST
Comment on attachment 335098 [details]
Patch

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

>> Source/WTF/wtf/Vector.h:71
>> +    static void initializeIfNecessary(T*, T*) { }
> 
> I am not sure what "if necessary" means here. I think a C++ expert would call this initializeUnlessConstructorIsTrivial or initializeIfConstructorIsNonTrivial.

Our Vector<> intentionally does not initialize backing store if T is POD (this is described in Vector.h constructor). This `initializeIfNecessary()` does that: do not initialize if T is POD.
`initializeIfNonPOD` would be fine I think. changed.

>> Source/WTF/wtf/VectorTraits.h:52
>> +        static const bool canInitializeWithMemset = std::is_integral<T>::value;
> 
> Why can’t we just set this to true? Are there types that return true for is_pod and can’t be initialized with memset?

I think it is OK: even `double` can be initialized with 0. Changed.
Comment 7 Yusuke Suzuki 2018-03-07 22:44:48 PST
Committed r229397: <https://trac.webkit.org/changeset/229397>
Comment 8 Radar WebKit Bug Importer 2018-03-08 09:29:08 PST
<rdar://problem/38266004>
Comment 9 Radar WebKit Bug Importer 2018-03-08 09:29:08 PST
<rdar://problem/38266005>