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

Should be able to generate spidermonkey bindings without downstream patches #16

Closed
fitzgen opened this issue Jul 18, 2016 · 6 comments
Closed

Comments

@fitzgen
Copy link
Member

fitzgen commented Jul 18, 2016

Right now, servo/mozjs has a bunch of downstream patches that simplify and dumb down spidermonkey headers and even completely hide various types.

We should get to the point where we don't need these downstream patches, and can run bindgen in CI for mozilla-central. That doesn't necessarily mean that we have to generate bindings for all of C++, but maybe have some configuration of things to ignore rather than manually ifdef away chunks of code.

Blocks https://bugzilla.mozilla.org/show_bug.cgi?id=1277343

@emilio
Copy link
Contributor

emilio commented Jul 18, 2016

Actually we have a flag to ignore unknown types, I don't think that's the problem.

The problem is most likely that we generate bad bindings for some kind of C++. We have an extensive set of flags to ignore types, mark them as opaque, etc. That's probably enough, except in the case we generate a bad type that is absolutely necessary to rust-mozjs.

Nick, do you know what are the blockers here? I'm happy to help :)

@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 19, 2016

Once the current smup lands, I'll try to see if we can replace some of those ifdefs by bindgen flags.

@fitzgen
Copy link
Member Author

fitzgen commented Jul 19, 2016

Does bindgen support keeping those flags in some kind of configuration file? Something we can keep in m-c/js/src/bindgen-flags or something? I think that would make keeping C-I green easier for spidermonkey hackers who don't necessarily want to touch bindgen/rust stuff when adding new JSAPI things.

@fitzgen
Copy link
Member Author

fitzgen commented Jul 19, 2016

Nick, do you know what are the blockers here? I'm happy to help :)

Thanks! :-D

Not yet, I have a couple things to get done first (grrr NSPR) but I was going to start just running bindgen on m-c's js/ and filing bugs / submitting PRs for things that fail without downstream patches.

Wash, rinse, repeat until everything Just Works :)

@emilio
Copy link
Contributor

emilio commented Jul 19, 2016

Sounds good to me :-)
Regarding the bindgen-flags thing, I don't think something like that would be difficult to implement.

For comparison, now what we have in stylo, for example, is a python script that takes care of invoking bindgen appropriately, and it works fairly well.

@emilio
Copy link
Contributor

emilio commented Apr 22, 2017

This is fixed, isn't it?

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

3 participants