Skip to content

FixedVector(size, value) should not double-initialize non-POD elements#61560

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
cdumez:310963_FixedVector_ctor
Mar 29, 2026
Merged

FixedVector(size, value) should not double-initialize non-POD elements#61560
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
cdumez:310963_FixedVector_ctor

Conversation

@cdumez
Copy link
Copy Markdown
Contributor

@cdumez cdumez commented Mar 28, 2026

422833b

`FixedVector(size, value)` should not double-initialize non-POD elements
https://bugs.webkit.org/show_bug.cgi?id=310963

Reviewed by Sam Weinig.

The `FixedVector(size_t, const T&)` constructor was first
default-constructing all elements via `Storage::create(size)`, then
copy-assigning over them with `fill(value)`. For non-POD types, this
means N default constructions + N copy assignments instead of just
N copy constructions.

Fix this by adding a `createFilled()` factory to `EmbeddedFixedVector`
backed by a new `TrailingArray` FillWith constructor that uses
`VectorTypeOperations::uninitializedFill`, matching what
`Vector(size, val)` already does.

Also add API test coverage for this constructor, which had none.

Test: Tools/TestWebKitAPI/Tests/WTF/FixedVector.cpp

* Source/WTF/wtf/EmbeddedFixedVector.h:
* Source/WTF/wtf/FixedVector.h:
(WTF::FixedVector::FixedVector):
* Source/WTF/wtf/TrailingArray.h:
(WTF::TrailingArray::TrailingArray):
* Tools/TestWebKitAPI/Tests/WTF/FixedVector.cpp:
(TestWebKitAPI::TEST(WTF_FixedVector, SizeAndValueConstructorPOD)):
(TestWebKitAPI::TEST(WTF_FixedVector, SizeAndValueConstructorNonPOD)):
(TestWebKitAPI::TEST(WTF_FixedVector, SizeAndValueConstructorZeroSize)):
(TestWebKitAPI::TEST(WTF_FixedVector, SizeAndValueConstructorSingleElement)):

Canonical link: https://commits.webkit.org/310167@main

96cea8f

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win loading 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ❌ 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-debug-arm64 ✅ 🛠 ios-safer-cpp ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@cdumez cdumez self-assigned this Mar 28, 2026
@cdumez cdumez marked this pull request as ready for review March 28, 2026 11:52
@@ -92,10 +92,8 @@ class FixedVector {
{ }

FixedVector(size_t size, const T& value)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be tempted to add a FillWith type tag here too, as the constructor without it is always a confusion point for me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fairly large change I suspect, especially because we'd want to align the similar Vector constructor as well. I will look into doing this separately.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdumez cdumez added the merge-queue Applied to send a pull request to merge-queue label Mar 29, 2026
https://bugs.webkit.org/show_bug.cgi?id=310963

Reviewed by Sam Weinig.

The `FixedVector(size_t, const T&)` constructor was first
default-constructing all elements via `Storage::create(size)`, then
copy-assigning over them with `fill(value)`. For non-POD types, this
means N default constructions + N copy assignments instead of just
N copy constructions.

Fix this by adding a `createFilled()` factory to `EmbeddedFixedVector`
backed by a new `TrailingArray` FillWith constructor that uses
`VectorTypeOperations::uninitializedFill`, matching what
`Vector(size, val)` already does.

Also add API test coverage for this constructor, which had none.

Test: Tools/TestWebKitAPI/Tests/WTF/FixedVector.cpp

* Source/WTF/wtf/EmbeddedFixedVector.h:
* Source/WTF/wtf/FixedVector.h:
(WTF::FixedVector::FixedVector):
* Source/WTF/wtf/TrailingArray.h:
(WTF::TrailingArray::TrailingArray):
* Tools/TestWebKitAPI/Tests/WTF/FixedVector.cpp:
(TestWebKitAPI::TEST(WTF_FixedVector, SizeAndValueConstructorPOD)):
(TestWebKitAPI::TEST(WTF_FixedVector, SizeAndValueConstructorNonPOD)):
(TestWebKitAPI::TEST(WTF_FixedVector, SizeAndValueConstructorZeroSize)):
(TestWebKitAPI::TEST(WTF_FixedVector, SizeAndValueConstructorSingleElement)):

Canonical link: https://commits.webkit.org/310167@main
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 310167@main (422833b): https://commits.webkit.org/310167@main

Reviewed commits have been landed. Closing PR #61560 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 422833b into WebKit:main Mar 29, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants