-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Fix pthreads + LIBRARY_DEBUG #8303
Conversation
tests/test_browser.py
Outdated
@@ -3665,7 +3665,8 @@ def test_pthread_condition_variable(self): | |||
# Test that pthreads are able to do printf. | |||
@requires_threads | |||
def test_pthread_printf(self): | |||
self.btest(path_from_root('tests', 'pthread', 'test_pthread_printf.cpp'), expected='0', args=['-s', 'TOTAL_MEMORY=64MB', '-O3', '-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=1']) | |||
for library_debug in [0, 1]: |
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.
I find that the backtrace in the error reporting is nicer if you follow the pattern and avoid loops like this in the test code:
def run(debug):
self.btest(path_from_root('tests', 'pthread', 'test_pthread_printf.cpp'), expected='0', args=['-s', 'TOTAL_MEMORY=64MB', '-O3', '-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=1', '-s', 'LIBRARY_DEBUG=%d' % debug])
run(debug=True)
run(debug=False)
src/parseTools.js
Outdated
assert(bodyEnd > 0); | ||
return func(name, args, rest.substring(bodyStart + 1, bodyEnd)); | ||
} | ||
|
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.
Remove trailing newline
assert(bodyStart > 0); | ||
var bodyEnd = rest.lastIndexOf('}'); | ||
assert(bodyEnd > 0); | ||
return func(name, args, rest.substring(bodyStart + 1, bodyEnd)); |
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.
I'm having trouble seeing there func
is defined. Does it return a string?
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.
I added a comment to clarify this now - basically we do the parsing here, and pass the parsed name, arguments, and body to the passed-in function which returns whatever it wants. (In the actual uses, it returns the string, but other uses could do otherwise.)
Both options modify the JS library functions, but LIBRARY_DEBUG did it in a destructive way (removed arguments from the declaration). This refactors both code locations to call a single helper function which does things properly, and adds a tes
This reverts commit a1e7f6f.
This reverts commit a1e7f6f.
Both options modify the JS library functions, but LIBRARY_DEBUG did it in a destructive way (removed arguments from the declaration). This refactors both code locations to call a single helper function which does things properly, and adds a test.