Skip to content

Commit

Permalink
Remove basic_string<unsigned char> from embind (emscripten-core#23070)
Browse files Browse the repository at this point in the history
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid
specializations for `std::basic_string`:
https://en.cppreference.com/w/cpp/string/basic_string

But libc++ had a base template for `basic_string` that allowed any type
to be passed for a long time. It looks there have been several attempts
to remove this after which they restored it due to complaints, in
chronological order:

llvm/llvm-project@aeecef0
llvm/llvm-project@08a0faf
llvm/llvm-project@e30a148
llvm/llvm-project#66153
llvm/llvm-project#72694

The last one, llvm/llvm-project#72694,
eventually removed it. So `std::basic_string<unsigned char>` is not
allowed anymore. This removes all uses of `std::basic_string<unsigned
char>` from embind.

This needs to be done to update libc++ to LLVM 19 (emscripten-core#22994). I'm
uploading this as a separate PR because this removes a functionality
from embind.
  • Loading branch information
aheejin authored Dec 11, 2024
1 parent f36fcb8 commit fb77f78
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 48 deletions.
4 changes: 4 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ See docs/process.md for more on how version tagging works.
- Emscripten-generated code will now use async/await internally when loading
the Wasm module. This will be lowered away by babel when targeting older
browsers. (#23068)
- Due to the discontinued support for invalid specializations of
- `std::basic_string` (https://github.com/llvm/llvm-project/pull/72694), the
support for `std::basic_string<unsigned char>` was removed from embind.
(#23070)

3.1.73 - 11/28/24
-----------------
Expand Down
3 changes: 1 addition & 2 deletions src/embind/embind.js
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,7 @@ var LibraryEmbind = {
name = readLatin1String(name);
var stdStringIsUTF8
#if EMBIND_STD_STRING_IS_UTF8
//process only std::string bindings with UTF8 support, in contrast to e.g. std::basic_string<unsigned char>
= (name === "std::string");
= true;
#else
= false;
#endif
Expand Down
2 changes: 0 additions & 2 deletions system/lib/embind/bind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,6 @@ EMSCRIPTEN_BINDINGS(builtin) {
register_float<double>("double");

_embind_register_std_string(TypeID<std::string>::get(), "std::string");
_embind_register_std_string(
TypeID<std::basic_string<unsigned char>>::get(), "std::basic_string<unsigned char>");
_embind_register_std_wstring(TypeID<std::wstring>::get(), sizeof(wchar_t), "std::wstring");
_embind_register_std_wstring(TypeID<std::u16string>::get(), sizeof(char16_t), "std::u16string");
_embind_register_std_wstring(TypeID<std::u32string>::get(), sizeof(char32_t), "std::u32string");
Expand Down
12 changes: 6 additions & 6 deletions test/code_size/embind_hello_wasm.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 552,
"a.html.gz": 380,
"a.js": 9718,
"a.js.gz": 4291,
"a.wasm": 7728,
"a.wasm.gz": 3502,
"total": 17998,
"total_gz": 8173
"a.js": 9593,
"a.js.gz": 4230,
"a.wasm": 7615,
"a.wasm.gz": 3471,
"total": 17760,
"total_gz": 8081
}
12 changes: 6 additions & 6 deletions test/code_size/embind_val_wasm.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 552,
"a.html.gz": 380,
"a.js": 6849,
"a.js.gz": 2947,
"a.wasm": 9568,
"a.wasm.gz": 4911,
"total": 16969,
"total_gz": 8238
"a.js": 6724,
"a.js.gz": 2900,
"a.wasm": 9528,
"a.wasm.gz": 4896,
"total": 16804,
"total_gz": 8176
}
27 changes: 0 additions & 27 deletions test/embind/embind.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,33 +449,6 @@ module({
assert.equal('ABCD', e);
});

test("can pass Uint8Array to std::basic_string<unsigned char>", function() {
var e = cm.emval_test_take_and_return_std_basic_string_unsigned_char(new Uint8Array([65, 66, 67, 68]));
assert.equal('ABCD', e);
});

test("can pass long string to std::basic_string<unsigned char>", function() {
var s = 'this string is long enough to exceed the short string optimization';
var e = cm.emval_test_take_and_return_std_basic_string_unsigned_char(s);
assert.equal(s, e);
});

test("can pass Uint8ClampedArray to std::basic_string<unsigned char>", function() {
var e = cm.emval_test_take_and_return_std_basic_string_unsigned_char(new Uint8ClampedArray([65, 66, 67, 68]));
assert.equal('ABCD', e);
});


test("can pass Int8Array to std::basic_string<unsigned char>", function() {
var e = cm.emval_test_take_and_return_std_basic_string_unsigned_char(new Int8Array([65, 66, 67, 68]));
assert.equal('ABCD', e);
});

test("can pass ArrayBuffer to std::basic_string<unsigned char>", function() {
var e = cm.emval_test_take_and_return_std_basic_string_unsigned_char((new Int8Array([65, 66, 67, 68])).buffer);
assert.equal('ABCD', e);
});

test("can pass string to std::string", function() {
var string = stdStringIsUTF8?"aeiáéíαειЖЛФ從獅子€":"ABCD";

Expand Down
5 changes: 0 additions & 5 deletions test/embind/embind_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,6 @@ std::string emval_test_take_and_return_std_string_const_ref(const std::string& s
return str;
}

std::basic_string<unsigned char> emval_test_take_and_return_std_basic_string_unsigned_char(std::basic_string<unsigned char> str) {
return str;
}

std::wstring take_and_return_std_wstring(std::wstring str) {
return str;
}
Expand Down Expand Up @@ -1914,7 +1910,6 @@ EMSCRIPTEN_BINDINGS(tests) {
//function("emval_test_take_and_return_const_char_star", &emval_test_take_and_return_const_char_star);
function("emval_test_take_and_return_std_string", &emval_test_take_and_return_std_string);
function("emval_test_take_and_return_std_string_const_ref", &emval_test_take_and_return_std_string_const_ref);
function("emval_test_take_and_return_std_basic_string_unsigned_char", &emval_test_take_and_return_std_basic_string_unsigned_char);
function("take_and_return_std_wstring", &take_and_return_std_wstring);
function("take_and_return_std_u16string", &take_and_return_std_u16string);
function("take_and_return_std_u32string", &take_and_return_std_u32string);
Expand Down

0 comments on commit fb77f78

Please sign in to comment.