Skip to content

Fix constructor embind#12973

Open
aminya wants to merge 2 commits into
emscripten-core:mainfrom
aminya:fix-constructor-embind
Open

Fix constructor embind#12973
aminya wants to merge 2 commits into
emscripten-core:mainfrom
aminya:fix-constructor-embind

Conversation

@aminya

@aminya aminya commented Dec 5, 2020

Copy link
Copy Markdown
Contributor

Description of the change

Adds back the removed method from this commit

Fixed Issues

Fixes #11274
Fixes #9704

Allows running Atom's core engine in the browser: atom/superstring#79

Minimal working example

This PR fixes the following
mwe.cpp

#include<emscripten/bind.h>

struct MyStruct {
  bool optionA;
  MyStruct(bool _optionA): optionA{_optionA} {};
};

MyStruct *constructor_(emscripten::val value) {
  bool optionA = false;
  if (value.as<bool>() && value["optionA"].as<bool>()) {
    optionA = true;
  }
  return new MyStruct(optionA);
}

EMSCRIPTEN_BINDINGS(MyStruct) {
  emscripten::class_<MyStruct>("MyStruct")
      .constructor<emscripten::val>(&constructor_);
}
em++ --bind -o mwe.js  -std=c++17  -O0 ./mwe.cpp  --memory-init-file 0

@welcome

welcome Bot commented Dec 5, 2020

Copy link
Copy Markdown

Thank you for submitting a pull request! If this is your first PR, make sure to add yourself to AUTHORS.

@aminya aminya force-pushed the fix-constructor-embind branch 3 times, most recently from 8946ea9 to 0227dc5 Compare December 6, 2020 14:05
@kripken

kripken commented Dec 8, 2020

Copy link
Copy Markdown
Member

@kripken kripken added the embind label Dec 8, 2020

@chadaustin chadaustin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As long as tests pass, seems okay to me, thanks!

@aminya aminya force-pushed the fix-constructor-embind branch from 0227dc5 to 2de498f Compare January 18, 2021 10:49
@aminya

aminya commented Jan 18, 2021

Copy link
Copy Markdown
Contributor Author

How can I run the embind tests in isolation so I can fix the issue?

@aminya aminya force-pushed the fix-constructor-embind branch from 2de498f to 08abc35 Compare January 18, 2021 11:37
@sbc100

sbc100 commented Jan 18, 2021

Copy link
Copy Markdown
Collaborator

You can run ./tests/runner.py other.test_embind. Or did you mean how can you just run one of the sub-tests within that? If you meant the latter I'm afraid I don't don't.. sadly the embind test is huge and runs a lot of tests within just one.

Base automatically changed from master to main March 8, 2021 23:49
@stale

stale Bot commented Apr 16, 2022

Copy link
Copy Markdown

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale Bot added the wontfix label Apr 16, 2022
@aminya

aminya commented Apr 17, 2022

Copy link
Copy Markdown
Contributor Author

I don't understand why the tests failed.

@stale stale Bot removed the wontfix label Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

implicit instantiation of undefined template Binding error when overload the constructor

4 participants