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

De @mut the bindings. #49

Merged
merged 1 commit into from Feb 28, 2014
Merged

De @mut the bindings. #49

merged 1 commit into from Feb 28, 2014

Conversation

@jdm
Copy link
Member

jdm commented Feb 25, 2014

No description provided.

@metajack

This comment has been minimized.

Copy link
Contributor

metajack commented on global.rs in f5b653e Feb 26, 2014

This API seems odd. Why limit it to statics?

This comment has been minimized.

Copy link
Member Author

jdm replied Feb 26, 2014

The string needs to stay alive forever, basically.

This comment has been minimized.

Copy link
Contributor

metajack replied Feb 26, 2014

It used to be owned.

If you transmute a ~str to something raw it won't be finalized unless you transmute it back and let it drop.

Also, now that I'm noticing, why is this all i8 instead of u8? :)

This comment has been minimized.

Copy link
Member Author

jdm replied Feb 26, 2014

It used to be owned when passed in, but then we stored it forever inside the name pool. I hate everything about that and it was awful; all serious JSAPI work I've done and seen uses static classes and strings everywhere. The u8/i8 thing is the libc::c_char is defined as *i8 for some reason.

This comment has been minimized.

Copy link
Contributor

metajack replied Feb 26, 2014

If normal JSAPI code uses static strings, that sounds fine then. Carry on.

@metajack

This comment has been minimized.

Copy link
Contributor

metajack commented on rust.rs in f5b653e Feb 26, 2014

Same 'static thing here.

This comment has been minimized.

Copy link
Member Author

jdm replied Feb 26, 2014

I don't actually know what the restrictions for the lifetime of the property name are for JS_DefineProperty. I'll take a look.

@metajack

This comment has been minimized.

Copy link
Contributor

metajack commented on f5b653e Feb 26, 2014

I don't like the statics. It seems pretty limiting, and it looks like there is some constraint I don't understand or this is just trying to save a clone or work around doing a transmute?

Looks fine otherwise.

bors-servo pushed a commit to servo/servo that referenced this pull request Feb 27, 2014
jdm added a commit that referenced this pull request Feb 28, 2014
@jdm jdm merged commit e89052b into master Feb 28, 2014
@jdm
Copy link
Member Author

jdm commented Feb 28, 2014

Durr, awkward. This didn't actually merge before the other, but worked because it's a branch of the official repo.

bors-servo pushed a commit to servo/servo that referenced this pull request Feb 28, 2014
bors-servo pushed a commit to servo/servo that referenced this pull request Feb 28, 2014
@Ms2ger Ms2ger deleted the demut branch Jan 28, 2015
mmatyas pushed a commit to mmatyas/rust-mozjs that referenced this pull request Jul 30, 2015
tschneidereit pushed a commit to tschneidereit/rust-mozjs that referenced this pull request Aug 26, 2017
Backport memory reporting changes from bug 1181452

The code is being backported from https://bugzilla.mozilla.org/show_bug.cgi?id=1181452. This change shouldn't land until the one in the Bugzilla bug has landed.
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Sep 30, 2019
…metajack

Requires servo/rust-mozjs#49.

Source-Repo: https://github.com/servo/servo
Source-Revision: ea29e3a001fb70b5e94af7a676345b4046771315

UltraBlame original commit: a989b2a4515b4ccc99680f910f27df7a2163f8db
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Sep 30, 2019
…metajack

Requires servo/rust-mozjs#49.

Source-Repo: https://github.com/servo/servo
Source-Revision: ea29e3a001fb70b5e94af7a676345b4046771315

UltraBlame original commit: a989b2a4515b4ccc99680f910f27df7a2163f8db
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
…metajack

Requires servo/rust-mozjs#49.

Source-Repo: https://github.com/servo/servo
Source-Revision: ea29e3a001fb70b5e94af7a676345b4046771315

UltraBlame original commit: a989b2a4515b4ccc99680f910f27df7a2163f8db
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.