Skip to content
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

ffi exports are not parsed correctly #35

Closed
karimsa opened this issue May 18, 2017 · 3 comments
Closed

ffi exports are not parsed correctly #35

karimsa opened this issue May 18, 2017 · 3 comments
Assignees

Comments

@karimsa
Copy link

karimsa commented May 18, 2017

It seems that at build time, when uses of ffi() are searched for in the JS code, the search also checks comments.

For instance, the following code can cause the build to error out:

// just to kill things: ffi('void non-existent-function(double)')

Which means that when debugging, if I want to comment out the ffi() use, I have to change the name of the function to ffi_ or anything else - I can't just comment it out.

Perhaps the defining of ffi_exports can be moved into a header file (i.e. main.h) or through main.c since they are converted to C anyways during the build process. Having the build process pull it out of the JS can lead to other issues as well and gives the illusion of dynamic interoperability - which isn't really the case anyways.

@karimsa
Copy link
Author

karimsa commented May 21, 2017

Why was this closed? Why is there no response at least if you're not interested in fixing this bug?

@dimonomid
Copy link
Contributor

Thanks for reporting!

The fix is committed to our internal repository, and will be published to https://github.com/cesanta/mongoose-os soon (technically, the issue is about mongoose-os, not the mjs itself). Now, comments are stripped from JS file before parsing ffi signatures.

Sorry for the confusion about closing it before publishing, github does that automatically because commit description mentions this issue.

@karimsa
Copy link
Author

karimsa commented May 21, 2017

Thanks! :)

cesantabot pushed a commit to cesanta/mongoose-os that referenced this issue May 21, 2017
Resolves cesanta/mjs#35

PUBLISHED_FROM=6f849e3f010d25f5acb28f3f54e73cfbeaa26ab2
dimonomid added a commit to cesanta/mos-tool that referenced this issue Jan 24, 2018
Resolves cesanta/mjs#35

PUBLISHED_FROM=6f849e3f010d25f5acb28f3f54e73cfbeaa26ab2
dimonomid added a commit to cesanta/mos-tool that referenced this issue Jan 25, 2018
Resolves cesanta/mjs#35

PUBLISHED_FROM=6f849e3f010d25f5acb28f3f54e73cfbeaa26ab2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants