-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add #if ASSERTIONS guard around assert in embind_shared #20504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r+ w/ nits fixed
Do you know why assert is undefined in this case? Can you share the command you run to generate that error? |
Adding the #if ASSERTIONS guard fixes the error ERROR - [JSC_NOT_FUNCTION_TYPE] { __clutz_actual_namespace: string, assert: function(*, (Error|string)=): undefined } expressions are not callable assert(signature[signature.length - 1] == ")", "Parentheses for argument names should match."); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
apply code review suggestions Co-authored-by: Brendan Dahl <brendan.dahl@gmail.com>
1c9080b
to
6ee793a
Compare
I believe the reason is that I'm compiling with The other flags I'm using are: PRINTF_LONG_DOUBLE=1 |
I'd love to see a test for this.. I'm trying to create a failing test locally, but not having much luck |
This matches the behaviour of MINIMAL_RUNTIME, and supports the practice of not including asserts in release builds. Also remove the closure externs for `assert` so that closure can correctly error out end `assert` is not defined. Helps with emscripten-core#20504
This matches the behaviour of MINIMAL_RUNTIME, and supports the practice of not including asserts in release builds. Also remove the closure externs for `assert` so that closure can correctly error out end `assert` is not defined. Helps with emscripten-core#20504
This matches the behaviour of MINIMAL_RUNTIME, and supports the practice of not including asserts in release builds. Also remove the closure externs for `assert` so that closure can correctly error out end `assert` is not defined. Helps with #20504
The fix landed in #20606 so I think we can close this one. |
Adding the #if ASSERTIONS guard fixes the error
ERROR - [JSC_NOT_FUNCTION_TYPE] {
__clutz_actual_namespace: string,
assert: function(*, (Error|string)=): undefined
} expressions are not callable
assert(signature[signature.length - 1] == ")", "Parentheses for argument names should match.");
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^