Skip to content

Commit

Permalink
Mark JsValue idx as NonZeroU32
Browse files Browse the repository at this point in the history
I noticed in the generated wasm for examples/dom that `JsStatic` takes two words of storage (for its `Option<JsValue>`) when it would seem to only need one, if we mandate that `0` is not a valid heap index. Changing the `.idx` type to `NonZeroU32` fixes this.

However, LLVM doesn't know that these values are non-zero yet (rust-lang/rust#49572) which means in the implementation of Option's `get_or_insert_with` (https://doc.rust-lang.org/nightly/src/core/option.rs.html#774) LLVM is no longer able to prove that the `unreachable!()` branch is never hit and thus JsStatic ends up pulling in all the panic infrastructure where it previously didn't. This results in the examples/dom output being much larger, rather than smaller as I had hoped. Probably makes sense to wait for the upstream issue to be fixed so that this can be landed without regressing anything.
  • Loading branch information
sophiebits committed Apr 21, 2018
1 parent 947386e commit 4e3fbf6
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 27 deletions.
5 changes: 3 additions & 2 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,15 +631,16 @@ impl<'a> Context<'a> {
if !self.exposed_globals.insert("slab") {
return;
}
self.global(&format!("let slab = [];"));
self.global(&format!("let slab = [null];"));
}

fn expose_global_slab_next(&mut self) {
if !self.exposed_globals.insert("slab_next") {
return;
}
self.global(&format!("
let slab_next = 0;
// Avoid 0 as an index to enable optimizing Option<JsValue> layout
let slab_next = 1;
"));
}

Expand Down
12 changes: 8 additions & 4 deletions src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use std::cell::UnsafeCell;
use std::marker::Unsize;
use std::mem::{self, ManuallyDrop};
use std::num::NonZeroU32;
use std::prelude::v1::*;

use JsValue;
Expand Down Expand Up @@ -95,9 +96,12 @@ impl<T> Closure<T>
///
/// This is the function where the JS closure is manufactured.
pub fn wrap(t: Box<T>) -> Closure<T> {
let idx = unsafe {
NonZeroU32::new_unchecked(!0)
};
Closure {
inner: UnsafeCell::new(t),
js: UnsafeCell::new(ManuallyDrop::new(JsValue { idx: !0 })),
js: UnsafeCell::new(ManuallyDrop::new(JsValue { idx })),
}
}

Expand All @@ -114,7 +118,7 @@ impl<T> Closure<T>
/// cleanup as it can.
pub fn forget(self) {
unsafe {
let idx = (*self.js.get()).idx;
let idx = (*self.js.get()).idx.get();
if idx != !0 {
super::__wbindgen_cb_forget(idx);
}
Expand Down Expand Up @@ -142,7 +146,7 @@ impl<'a, T> IntoWasmAbi for &'a Closure<T>
unsafe {
let fnptr = WasmClosure::into_abi(&mut **self.inner.get(), extra);
extra.push(fnptr);
&mut (*self.js.get()).idx as *const u32 as u32
&mut (*self.js.get()).idx.get() as *const u32 as u32
}
}
}
Expand All @@ -162,7 +166,7 @@ impl<T> Drop for Closure<T>
{
fn drop(&mut self) {
unsafe {
let idx = (*self.js.get()).idx;
let idx = (*self.js.get()).idx.get();
if idx != !0 {
super::__wbindgen_cb_drop(idx);
}
Expand Down
9 changes: 5 additions & 4 deletions src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//! at your own risk.
use core::mem::{self, ManuallyDrop};
use core::num::NonZeroU32;
use core::ops::{Deref, DerefMut};
use core::slice;
use core::str;
Expand Down Expand Up @@ -242,7 +243,7 @@ impl IntoWasmAbi for JsValue {
type Abi = u32;

fn into_abi(self, _extra: &mut Stack) -> u32 {
let ret = self.idx;
let ret = self.idx.get();
mem::forget(self);
return ret
}
Expand All @@ -252,14 +253,14 @@ impl FromWasmAbi for JsValue {
type Abi = u32;

unsafe fn from_abi(js: u32, _extra: &mut Stack) -> JsValue {
JsValue { idx: js }
JsValue { idx: NonZeroU32::new_unchecked(js) }
}
}

impl<'a> IntoWasmAbi for &'a JsValue {
type Abi = u32;
fn into_abi(self, _extra: &mut Stack) -> u32 {
self.idx
self.idx.get()
}
}

Expand All @@ -268,7 +269,7 @@ impl RefFromWasmAbi for JsValue {
type Anchor = ManuallyDrop<JsValue>;

unsafe fn ref_from_abi(js: u32, _extra: &mut Stack) -> Self::Anchor {
ManuallyDrop::new(JsValue { idx: js })
ManuallyDrop::new(JsValue { idx: NonZeroU32::new_unchecked(js) })
}
}

Expand Down
35 changes: 18 additions & 17 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
//! this crate and this crate also provides JS bindings through the `JsValue`
//! interface.
#![feature(use_extern_macros, wasm_import_module, try_reserve, unsize)]
#![feature(use_extern_macros, wasm_import_module, try_reserve, unsize, nonzero)]
#![no_std]

extern crate wasm_bindgen_macro;

use core::cell::UnsafeCell;
use core::num::NonZeroU32;
use core::ops::Deref;
use core::ptr;

Expand Down Expand Up @@ -52,7 +53,7 @@ if_std! {
/// will transfer into wasm directly and this will likely become more efficient,
/// but for now it may be slightly slow.
pub struct JsValue {
idx: u32,
idx: NonZeroU32,
}

impl JsValue {
Expand All @@ -62,7 +63,7 @@ impl JsValue {
/// be owned by the JS garbage collector.
pub fn from_str(s: &str) -> JsValue {
unsafe {
JsValue { idx: __wbindgen_string_new(s.as_ptr(), s.len()) }
JsValue { idx: NonZeroU32::new_unchecked(__wbindgen_string_new(s.as_ptr(), s.len())) }
}
}

Expand All @@ -72,7 +73,7 @@ impl JsValue {
/// allocated number) and returns a handle to the JS version of it.
pub fn from_f64(n: f64) -> JsValue {
unsafe {
JsValue { idx: __wbindgen_number_new(n) }
JsValue { idx: NonZeroU32::new_unchecked(__wbindgen_number_new(n)) }
}
}

Expand All @@ -82,21 +83,21 @@ impl JsValue {
/// allocated boolean) and returns a handle to the JS version of it.
pub fn from_bool(b: bool) -> JsValue {
unsafe {
JsValue { idx: __wbindgen_boolean_new(b as u32) }
JsValue { idx: NonZeroU32::new_unchecked(__wbindgen_boolean_new(b as u32)) }
}
}

/// Creates a new JS value representing `undefined`.
pub fn undefined() -> JsValue {
unsafe {
JsValue { idx: __wbindgen_undefined_new() }
JsValue { idx: NonZeroU32::new_unchecked(__wbindgen_undefined_new()) }
}
}

/// Creates a new JS value representing `null`.
pub fn null() -> JsValue {
unsafe {
JsValue { idx: __wbindgen_null_new() }
JsValue { idx: NonZeroU32::new_unchecked(__wbindgen_null_new()) }
}
}

Expand All @@ -108,7 +109,7 @@ impl JsValue {
unsafe {
let ptr = description.map(|s| s.as_ptr()).unwrap_or(ptr::null());
let len = description.map(|s| s.len()).unwrap_or(0);
JsValue { idx: __wbindgen_symbol_new(ptr, len) }
JsValue { idx: NonZeroU32::new_unchecked(__wbindgen_symbol_new(ptr, len)) }
}
}

Expand Down Expand Up @@ -137,7 +138,7 @@ impl JsValue {
pub fn as_f64(&self) -> Option<f64> {
let mut invalid = 0;
unsafe {
let ret = __wbindgen_number_get(self.idx, &mut invalid);
let ret = __wbindgen_number_get(self.idx.get(), &mut invalid);
if invalid == 1 {
None
} else {
Expand All @@ -155,7 +156,7 @@ impl JsValue {
pub fn as_string(&self) -> Option<String> {
unsafe {
let mut len = 0;
let ptr = __wbindgen_string_get(self.idx, &mut len);
let ptr = __wbindgen_string_get(self.idx.get(), &mut len);
if ptr.is_null() {
None
} else {
Expand All @@ -172,7 +173,7 @@ impl JsValue {
/// `None`.
pub fn as_bool(&self) -> Option<bool> {
unsafe {
match __wbindgen_boolean_get(self.idx) {
match __wbindgen_boolean_get(self.idx.get()) {
0 => Some(false),
1 => Some(true),
_ => None,
Expand All @@ -183,21 +184,21 @@ impl JsValue {
/// Tests whether this JS value is `null`
pub fn is_null(&self) -> bool {
unsafe {
__wbindgen_is_null(self.idx) == 1
__wbindgen_is_null(self.idx.get()) == 1
}
}

/// Tests whether this JS value is `undefined`
pub fn is_undefined(&self) -> bool {
unsafe {
__wbindgen_is_undefined(self.idx) == 1
__wbindgen_is_undefined(self.idx.get()) == 1
}
}

/// Tests whether the type of this JS value is `symbol`
pub fn is_symbol(&self) -> bool {
unsafe {
__wbindgen_is_symbol(self.idx) == 1
__wbindgen_is_symbol(self.idx.get()) == 1
}
}
}
Expand Down Expand Up @@ -269,16 +270,16 @@ extern {
impl Clone for JsValue {
fn clone(&self) -> JsValue {
unsafe {
let idx = __wbindgen_object_clone_ref(self.idx);
JsValue { idx }
let idx = __wbindgen_object_clone_ref(self.idx.get());
JsValue { idx: NonZeroU32::new_unchecked(idx) }
}
}
}

impl Drop for JsValue {
fn drop(&mut self) {
unsafe {
__wbindgen_object_drop_ref(self.idx);
__wbindgen_object_drop_ref(self.idx.get());
}
}
}
Expand Down

0 comments on commit 4e3fbf6

Please sign in to comment.