-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add iterable/iterator support for wasm-bindgen
.
#1036
Comments
I'm not sure how this interacts with |
I don't think we need macro support for this, a wrapper like this should work (untested): struct Iterable<A> {
value: js_sys::Iterator,
phantom: PhantomData<A>,
}
impl<A> for Iterable<A> {
fn new(value: &JsValue) -> Result<Self, JsValue> {
let value = js_sys::Reflect::get(value, js_sys::Symbol::iterator().as_ref())?;
let value = js_sys::Reflect::apply(value.unchecked_ref(), &value, &js_sys::Array::new())?;
Self {
value: value.into(),
phantom: PhantomData,
}
}
}
impl<A> Iterator for Iterable<A> {
type Item = A;
#[inline]
fn next(&mut self) -> Option<Self::Item> {
self.value.next().map(|x| {
JsCast::unchecked_from_js(x.unwrap())
})
}
} It can be made more efficient (e.g. by creating some bindings that lookup and call And now you can easily make other types Iterable: impl NodeList {
#[inline]
fn iter(&self) -> Iterable<Node> {
Iterable::new(self.as_ref()).unwrap()
}
}
impl IntoIterator for NodeList {
type Item = Node;
type IntoIter = Iterable<Self::Item>;
#[inline]
fn into_iter(self) -> Self::IntoIter {
self.iter()
}
} Notice that it returns a As its name implies, this is unchecked, so there are no runtime checks (it is zero-cost), but that means it's up to you to ensure that the static types are correct. |
(To be clear, I think having |
I think the best strategy might be to have an |
Do you mean to replace |
I think it's probably worthwhile to have a new one in |
@alexcrichton It should be in Also, we can just name it something else to avoid conflict with |
Hm actually alternatively I think the best option here may be to generate methods that return |
I guess that's kinda what I'm pushing for
…On Mon, 19 Nov 2018, 18:03 Alex Crichton ***@***.*** wrote:
Hm actually alternatively I think the best option here may be to generate
methods that return impl Iterator<Item = ...>, it'd be great to not have
to deal with naming concerns
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1036 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABU-Xj2HtGgyiNDFox25z8xkxaT9qF_5ks5uwvKFgaJpZM4YnfS9>
.
|
@alexcrichton Sure, that's fine for methods like
So we still need to decide on a name. I think |
Here is my understanding of the existing iterator types in js-sys:
So we could introduce some more types here for where we know the type pub struct TypedIterator<T> {
js: Iterator,
state: IterState,
ty: PhantomData<T>,
} that panics when the type is incorrect. Is this what you prefer? Or should these types live in web-sys, since in js you can never trust types except when using with webidl? |
Also would you consider renaming these types to maybe
And mention in the docs that you don't really need to look at these types at all - they are just there as things that There is also |
That sounds correct.
In the general case I don't think we can panic when the type is incorrect (since JS doesn't have a general-purpose type-checking feature). I think using I do like the name @alexcrichton Do you have a different opinion about this?
To reiterate: this should not live in web-sys, because any JS object can implement the Iterable protocol, it's not WebIDL-specific. As for the types, that's not a problem, it can just have an
Oh! I didn't realize struct Iterable<A> {
value: js_sys::IntoIter,
phantom: PhantomData<A>,
}
impl<A> for Iterable<A> {
fn unchecked_new(value: &JsValue) -> Self {
Self {
value: js_sys::try_iter(value).unwrap().unwrap(),
phantom: PhantomData,
}
}
}
impl<A> Iterator for Iterable<A> {
type Item = A;
#[inline]
fn next(&mut self) -> Option<Self::Item> {
self.value.next().map(|x| {
JsCast::unchecked_from_js(x.unwrap())
})
}
} Though |
@Pauan I feel like we're both moving towards consensus. I just have a few more questions:
|
Sorry I've been a bit slow to catch up on this, but are there still unresolved questions for how we might expose this? |
They are both kinda supported already - there is a |
Friendly asking if there's any progress on this? Is the related PR still valid? |
Fixed by #3962. |
EDIT I'm going to write some notes for myself about iterators in ECMAScript and webidl. The original content of the issue follows below.
Iterator
s anditerable
s in ECMAScriptIn ECMAScript, there are 2 special interfaces that objects can implement,
iterator
anditerable
. A type is aniterator
if it has a method callednext
that returns an object of the form{ done: boolean, value: T }
for any type T that ECMAScript supports.A type is
iterable
if it has a property with the keySymbol.iterator
, and the value of that property is aniterator
.These 2 types match nicely to
Iterator
andFromIterator
in rust, and work in a similar way.Iterable
s in webidlIn webidl there is also the concept of an
iterable
that is different to bothiterator
s anditerable
s in ECMAScript. In webidl, an iterable is an array-type or a map-type. It has theSymbol.iterator
key, so it is an ECMAScript iterable, but also 4 methods:entries
,forEach
,keys
, andvalues
.The semantics of these functions are:
entries
- Aniterator
over arrays of length 2, where the first value is the key (or index in the case of array-like iterables), and the second value is the value.forEach
- Run the supplied function on each value - in the case of map-like keys are discarded.keys
- Aniterator
over the keys or indices of theiterable
, depending on whether it is array-like or map-likevalues
- Aniterator
over the values of theiterable
, discarding keys.If the iterable is array-type, the iterator at
Symbol.iterator
is the same asvalues
, whereas if the iterable is map-type, then the iterator atSymbol.iterator
is the same asentries
.Original issue
I've been playing with solutions to #514, but I think the neatest is to have support for iterables/iterators within wasm-bindgen. It would work something like the following:
For now, the binding is for imports only (js -> rust). It would be possible the other way as well if that were desirable.
An imported type can be marked as
iterator
, something like the followingwhich would mean that the type
MyIterator
has a methodnext
in js, returning an object{ done: bool, value: OtherType }
.We use this information to generate
There would be some work to do behind the scenes, things like checking there isn't already a method called
next
on the type, then adding one and generating/using a struct of the formthat the
next
method will return.Then for
iterable
, we need to eitheriterator
in js,IntoIterator::IntoIter
in rust) as above and then implementIntoIterator
for theiterable
, or#[wasm_bindgen]
attribute specifying which typeIntoIter
is.For the
iterable
option, we need to generate the js code for indexing the object atSymbol.iterator
(obj[Symbol.iterator]
).Ok so this is really an RFC, I don't think I should just implement this because it's making some design choices that should be agreed (or not). I'm also happy to implement it, but may need some mentoring, especially around the actual wasm-bindgen tool, as I've only worked with the compiler plugin so far.
EDIT There is an alternative approach to the
fn next(&mut self) -> Option<Self::Item>
: we could do the conversion in javascript - so during rust compilation we insert something that the cli tool picks up, and actually implements this method on the js side, thereby avoiding the need for the IteratorNext type.The text was updated successfully, but these errors were encountered: