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

Fix signature of JS_BufferIsCompilableUnit #192

Closed
wants to merge 1 commit into from

Conversation

@tschneidereit
Copy link
Contributor

tschneidereit commented Aug 20, 2015

*mut JSContext,
obj:
Handle<*mut JSObject>,
utf8:
*const i8,
length:
u32)
u64)

This comment has been minimized.

@Manishearth

Manishearth Aug 20, 2015

Member

This is size_t. Should we be be using usize?

This comment has been minimized.

@tschneidereit

tschneidereit Aug 20, 2015

Author Contributor

This is size_t. Should we be be using usize?

Probably, yes. It seems like bindgen generated wrong code for size_t? I
wonder if that afflicts more sites than just this one.

This comment has been minimized.

This comment has been minimized.

@Ms2ger

Ms2ger Aug 20, 2015

Collaborator

Yes and yes: #185

This comment has been minimized.

@Manishearth

Manishearth Aug 20, 2015

Member

Is libc::size_t different from usize? It would be nice to use Rust ints whenever possible.

@michaelwu
Copy link
Contributor

michaelwu commented Aug 20, 2015

https://gist.github.com/michaelwu/a9525f07112a8224a540 is the latest output from bindgen. size_t is handled correctly now.

Replacing size_t with usize sounds good - I'll try that next time, though I think we should stick to size_t for now for consistency.

@michaelwu

This comment has been minimized.

Copy link

michaelwu commented on src/jsapi.rs in 4211969 Aug 20, 2015

This needs to be size_t.

@michaelwu

This comment has been minimized.

Copy link

michaelwu commented on src/jsapi.rs in 4211969 Aug 20, 2015

Shouldn't work everywhere. Since this is using size_t, this particular symbol likely to be mangled in three different ways. One for 32bit systems, one for 64 bit Linux, and another for 64 bit OSX. This is why many functions that use size_t have three different versions, controlled by cfg attributes.

@michaelwu
Copy link
Contributor

michaelwu commented Sep 24, 2015

This should be fixed by the bindings update.

@michaelwu
Copy link
Contributor

michaelwu commented Oct 7, 2015

#198 will fix this.

@michaelwu michaelwu closed this Oct 7, 2015
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.