Skip to content

Add std::inplace_vector polyfill from c++26 [NFC]#8814

Merged
kripken merged 10 commits into
WebAssembly:mainfrom
kripken:inplace
Jun 8, 2026
Merged

Add std::inplace_vector polyfill from c++26 [NFC]#8814
kripken merged 10 commits into
WebAssembly:mainfrom
kripken:inplace

Conversation

@kripken

@kripken kripken commented Jun 8, 2026

Copy link
Copy Markdown
Member

Header is basically our SmallVector, modified to remove the dynamic
allocation.

This is already useful in one place, and will help more with a future PR.

@kripken kripken requested a review from a team as a code owner June 8, 2026 21:17
@kripken kripken requested review from aheejin and removed request for a team June 8, 2026 21:17
Comment thread src/support/inplace_vector.h Outdated

#include "support/parent_index_iterator.h"

namespace std {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's UB in most cases to add things to std, we should not do that here. Not to mention there are several behavioral differences between this implementation and the standard, especially around when constructors and destructors are called on the elements.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok, done.

EXPECT_EQ(vec[0], 10);

vec.resize(3);
EXPECT_EQ(vec.size(), 3);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should also check that the size is 0 and then 1 before doing this resize.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

Comment thread src/support/inplace_vector.h Outdated
inplace_vector(inplace_vector<T, N>&& other)
: usedFixed(other.usedFixed), fixed(std::move(other.fixed)) {}
inplace_vector(std::initializer_list<T> init) {
for (T item : init) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we loop through const T& to avoid a copy here? It looks like we're doing 2 copies per element here (once in the loop iteration and a second one in push_back).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good idea, this is copied from SmallVector but I'll improve it there too.

@kripken

kripken commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

Also the fuzzer found I misread the CFP code - the size is not limited to 2 there (each call to handleType is limited, but we call it multiple times). So this adds no uses of the class, for now.

@kripken kripken changed the title Add std::inplace_vector polyfill from c++26, and use it in one place Add std::inplace_vector polyfill from c++26 [NFC] Jun 8, 2026
@kripken kripken merged commit cacc711 into WebAssembly:main Jun 8, 2026
16 checks passed
@kripken kripken deleted the inplace branch June 8, 2026 23:32
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.

3 participants