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

Upgrade to SM 39 #144

Closed
wants to merge 2 commits into from
Closed

Upgrade to SM 39 #144

wants to merge 2 commits into from

Conversation

@michaelwu
Copy link
Contributor

michaelwu commented Apr 24, 2015

Some things have been implemented in rust.rs. Where possible, things are being implemented using the structs generated by my bindgen fork (which I will call bindgen++ from now on).

jsapi.rs has been hand edited a bit. Note that jsapi.rs is generated from jsfriendapi.hpp so we can eliminate jsfriendapi.rs entirely. Making jsfriendapi.rs work with with jsapi.rs was a bit difficult, so I combined them to sidestep the problem.

Here are the things I had to hand edit in jsapi.rs:

  • Adding "unsafe impl Sync for " for structs that are statically defined.
  • Adding "pub enum Foo {}" for types not defined in js headers. (usually mfbt)
  • Adding "#[derive(Copy)]" where necessary to those defines.
  • Removing derive copy from autogenerated structs that don't support it because bindgen++ doesn't fully define all the structs it depends on.
  • Trimming away generics for structs that didn't get converted successfully. This can happen for a number of reasons:
    • bindgen++ can only convert templates that work on types
    • bindgen++ can't convert the classes that a struct/class might inherit from
  • 64 bit issues:
    • Certain types need to be converted back to size_t since bindgen++ currently replaces them with u32 or u64. This will be fixed.
    • Some mangled function names are different between 32 bit and 64 bit, usually because of 64 bit types and size_t/long. In these cases, functions are hand edited to add 64bit versions. Might be fixed in the future, but for now we can only detect these problems when code doesn't link.
  • Spidermonkey checks for the address of JS_GlobalObjectTraceHook in globals. For now, I made the mangled name public so we can grab the real function address, but we can probably do a little bit better.
  • References to JSVal are replaced with Value, since jsapi.rs and jsval.rs both define JSVal. In jsapi.rs, JSVal is just a typedef to Value, so we can drop JSVal in jsapi.rs.
  • bindgen++ doesn't support preprocessor defines at the moment, so some defines were manually added.
  • JSJitMethodCallArgs was manually defined.
  • JS_GetRuntime wasn't converted by bindgen++, so it was manually added. Need to investigate.
  • Structs with unions generated by bindgen++ are generally unusable for statically defined things, so the necessary structs were modified to be more friendly for that case.
@jdm
Copy link
Member

jdm commented Apr 24, 2015

That sounds exactly like what I ran into for Root vs Root<*mut T> in the previous upgrade attempt.

@michaelwu michaelwu force-pushed the michaelwu:smupgrade2 branch 4 times, most recently from 29082a1 to 9dbf6fe May 1, 2015
@michaelwu michaelwu force-pushed the michaelwu:smupgrade2 branch from 9dbf6fe to 26a3628 May 14, 2015
@michaelwu michaelwu closed this May 21, 2015
@michaelwu michaelwu mentioned this pull request May 21, 2015
bors-servo pushed a commit that referenced this pull request Jun 18, 2015
Upgrade to SM 39

Some things have been implemented in rust.rs. Where possible, things are being implemented using the structs generated by my bindgen fork (which I will call bindgen++ from now on).

jsapi.rs has been hand edited a bit. Note that jsapi.rs is generated from jsfriendapi.hpp so we can eliminate jsfriendapi.rs entirely. Making jsfriendapi.rs work with with jsapi.rs was a bit difficult, so I combined them to sidestep the problem.

Here are the things I had to hand edit in jsapi.rs:
- Adding "unsafe impl Sync for " for structs that are statically defined.
- Adding "pub enum Foo {}" for types not defined in js headers. (usually mfbt)
- Adding "#[derive(Copy)]" where necessary to those defines.
- Removing derive copy from autogenerated structs that don't support it because bindgen++ doesn't fully define all the structs it depends on.
- Trimming away generics for structs that didn't get converted successfully. This can happen for a number of reasons:
  - bindgen++ can only convert templates that work on types
  - bindgen++ can't convert the classes that a struct/class might inherit from
- 64 bit issues:
  - Certain types need to be converted back to size_t since bindgen++ currently replaces them with u32 or u64. This will be fixed.
  - Some mangled function names are different between 32 bit and 64 bit, usually because of 64 bit types and size_t/long. In these cases, functions are hand edited to add 64bit versions. Might be fixed in the future, but for now we can only detect these problems when code doesn't link.
- Spidermonkey checks for the address of JS_GlobalObjectTraceHook in globals. For now, I made the mangled name public so we can grab the real function address, but we can probably do a little bit better.
- References to JSVal are replaced with Value, since jsapi.rs and jsval.rs both define JSVal. In jsapi.rs, JSVal is just a typedef to Value, so we can drop JSVal in jsapi.rs.
- bindgen++ doesn't support preprocessor defines at the moment, so some defines were manually added.
- JSJitMethodCallArgs was manually defined.
- JS_GetRuntime wasn't converted by bindgen++, so it was manually added. Need to investigate.
- Structs with unions generated by bindgen++ are generally unusable for statically defined things, so the necessary structs were modified to be more friendly for that case.

#144
mmatyas pushed a commit to mmatyas/rust-mozjs that referenced this pull request Jul 30, 2015
Add Crates dependency for libc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.