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

Regenerate bindings using the latest bindgen++ #198

Merged
merged 2 commits into from Oct 8, 2015

Conversation

@michaelwu
Copy link
Contributor

michaelwu commented Sep 24, 2015

Need to test this a bit more, but it's structured roughly the way I want it and no manual edits of jsapi_*.rs are required.

Fixes #158

Review on Reviewable

@michaelwu michaelwu force-pushed the michaelwu:update-bindings branch from 83ac87d to b567989 Sep 24, 2015
@michaelwu
Copy link
Contributor Author

michaelwu commented Sep 24, 2015

Tested and working on 64 bit OSX and 64 bit Linux. Had to fix an issue with the mangled names on OSX where an extra underscore was being prefixed to the mangled name.

@michaelwu
Copy link
Contributor Author

michaelwu commented Sep 25, 2015

Tested and working on Gonk. Android is the only one left to check, I think.

@michaelwu
Copy link
Contributor Author

michaelwu commented Sep 25, 2015

Adjusted the bindings generation a bit after @larsbergstrom found missing symbols on Android. Need to test once more, but I think this is ready for review. r? @jdm

@michaelwu
Copy link
Contributor Author

michaelwu commented Sep 25, 2015

Works on Android.

@michaelwu michaelwu force-pushed the michaelwu:update-bindings branch from f73410d to 5fb9b7c Oct 5, 2015
@michaelwu michaelwu force-pushed the michaelwu:update-bindings branch from 5fb9b7c to 65da425 Oct 8, 2015
@jdm
Copy link
Member

jdm commented Oct 8, 2015

<jdm> but yeah, I'd be inclined to rewrite all of the generated transmutes to be pointer casts instead
<jdm> let raw = &mut self._bindgen_data_1_ as *mut _ as *mut u8; raw.offset(0) as *mut _
src/rust.rs Outdated
HandleValue {
ptr: &JSVAL_NULL
unsafe {
HandleValue::from_marked_location(&JSVAL_NULL)
}

This comment has been minimized.

@jdm

jdm Oct 8, 2015

Member

Let's use NullHandleValue.

src/rust.rs Outdated
HandleValue {
ptr: &JSVAL_VOID
unsafe {
HandleValue::from_marked_location(&JSVAL_VOID)
}

This comment has been minimized.

@jdm

jdm Oct 8, 2015

Member

UndefinedHandleValue.

src/rust.rs Outdated
@@ -508,35 +543,41 @@ impl JSJitMethodCallArgs {
// to duplicate so much code here
impl CallArgs {
pub fn from_vp(vp: *mut Value, argc: u32) -> CallArgs {
use jsapi::{CallArgsBase, CallReceiverBase, IncludeUsedRval, UsedRvalBase};

This comment has been minimized.

@jdm

jdm Oct 8, 2015

Member

I'd prefer this up with the other includes.

@jdm
Copy link
Member

jdm commented Oct 8, 2015

* jdm is curious why FunctionClassPtr is a Class, while Int8ArrayClassPtr is a *const Class
 <mwu> jdm: looks like a bug. I think they should all be *const Class
<mwu> though.. this code is pretty clever, so I dunno
<mwu> probably a bug.
src/rust.rs Outdated

reserved: [0 as *mut libc::c_void; 25]
};

unsafe { assert_eq!(JS_Init(), 1); }
unsafe { assert_eq!(JS_Init(), true); }

This comment has been minimized.

@jdm

jdm Oct 8, 2015

Member

This could just be unsafe { assert!(JS_Init()); } now.

@jdm
Copy link
Member

jdm commented Oct 8, 2015

Ok, I've pointed out all of the things in the generated output for linux32 that made me scratch my head, and the stuff in the non-generated files that could be improved, so I think my part here is complete.

@michaelwu michaelwu force-pushed the michaelwu:update-bindings branch from e1e75a6 to 9e76ea6 Oct 8, 2015
@jdm
Copy link
Member

jdm commented Oct 8, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

📌 Commit 9e76ea6 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

Testing commit 9e76ea6 with merge 01ff3cb...

bors-servo pushed a commit that referenced this pull request Oct 8, 2015
Regenerate bindings using the latest bindgen++

Need to test this a bit more, but it's structured roughly the way I want it and no manual edits of jsapi_*.rs are required.

Fixes #158

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/rust-mozjs/198)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 8, 2015

☀️ Test successful - travis

@bors-servo bors-servo merged commit 9e76ea6 into servo:master Oct 8, 2015
1 check was pending
1 check was pending
homu Testing commit 9e76ea6 with merge 01ff3cb...
Details
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

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