Fix for register_vector and optional class_ collision. #21957
Conversation
wffurr
force-pushed
the
register-optional-check-twice
branch
from
fb3011e
to
ebd2523
Compare
Fixes `BindingError: Cannot register type 'emscripten::val' twice` error. This happened with pre-existing binding code that was migrated to Emscripten 3.1.52 which introduces the register_optional function, used internally by register_vector. The fix ignores duplicate registrations of emscripten::val, and doesn't rely on the boolean already-registered flag local to the register_optional function.
wffurr
force-pushed
the
register-optional-check-twice
branch
from
ebd2523
to
05ea0d0
Compare
|
Tests pass except for the binary size, +7 bytes. Please take a look and let me know if this change is a good one and if so I'll fix that test. |
Rebasing/merging should fix those failure. They are related I believe. |
| return; | ||
| } | ||
| hasRun = true; | ||
| // Optional types are automatically registered for some internal types; the |
There was a problem hiding this comment.
Allowing duplicate registrations will lead to a slightly strange behavior where the order matters. e.g.
The following will fail:
register_optional<Foo*>();
class_<std::optional<Foo*>>("OptionalFoo");but swapping them will work:
class_<std::optional<Foo*>>("OptionalFoo");
register_optional<Foo*>();I guess if a user is creating their own binding for std::optional and they want to override the behavior that could be fine.
|
This PR also doesn't fully address backwards incompatibility between |
Fixes
BindingError: Cannot register type 'emscripten::val' twiceerror. This happened with pre-existing binding code that was migrated to Emscripten 3.1.52 which introduces the register_optional function, used internally by register_vector, in #21076.The fix ignores duplicate registrations of emscripten::val, and doesn't rely on the boolean already-registered flag local to the register_optional function.