-
Notifications
You must be signed in to change notification settings - Fork 953
stack overflow building wasm #1848
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
Comments
I can confirm this issue. In the dev branch, there is also a different bug that triggers before the infinite recursion in this issue which I have a fix for. |
…#1848) and cleaned up the format
This is still happening here on the dev branch (v0.18.0-78-ge02f308), and is blocking compilation of a real app. |
Another test case, boiled down from the app I was trying to build:
|
If I had to guess it's this line that is causing trouble: https://github.com/yuin/goldmark/blob/master/util/util.go#L765 var emailDomainRegexp = regexp.MustCompile(`^[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*`) And in the case of github.com/cloudflare/ahocorasick, a quick glance at the code of |
Right now, fixing this is a bit complicated because of the special casing in here: Lines 793 to 801 in 1646326
But perhaps it's still possible without much more refactoring. |
Thanks for the update! Fortunately, I was able to pin an earlier version of a dependency in my app and avoid the explosion for now, but it's tech debt we'd like to address sooner or later. (Plus, we control that dependency, and if using sync.Once lazy initialization fixes things, we're probably ok.) |
I have a fix for this bug that I will submit shortly. It's relatively big. |
Finishing this fix took a little bit longer than expected. It definitely works, I just need to polish it and add tests etc. so it's reviewable. |
The PR #2001 fixes the test case in #1848 (comment) but sadly doesn't yet fix the issue in the issue description. |
Tested PR #2001, verified that it fixes the app I boiled down the ahocorasick test case from. |
Oopie! I don't think this is over just yet :Z Here is my program:
Go runs as normal:
But TinyGo does not:
Now if I simplify the program to:
I then got,
I also tried setting
|
@fgsch Thanks for the info. So I tried the code in
Not sure it's because my setup or something else. I'll test it again after the PR is completed. Thank you again! |
I think precise-alloc has regressed. Try the old version,
|
Try current dev branch, the regression was fixed... that said, initializing a complex regexp at init time |
@nirui can you close this issue? It looks like you were able to succeed (due to 👍 ) and I also just tried code in your comment and was able to run #1848 (comment) |
Hi @codefromthecrypt, I can confirm the code is working. But I'm not really sure how to close this issue (I don't have permission to do so). Maybe give @mayocream the raiser a ping would help? |
indeed you are right @nirui! @mayocream do you mind closing this out? |
Thanks for the follow up, i'll close this issue as it was resolved. 😀 |
Uh oh!
There was an error while loading. Please reload this page.
failed building wasm importing package "github.com/yuin/goldmark/parser"
test code:
The text was updated successfully, but these errors were encountered: