Refactor WebIDL code generation

This commit refactors WebIDL code generation to walk over the fields of
`FirstPassRecord` instead of walking the AST again. This helps remove
redundancies like checking `is_chrome_only` as well as revisiting partial
interfaces and such. This should make it more clear that the first pass's job is
to walk the AST and collect all relevant information, while the codegen pass is
purely about appending items to a `Program`.

Additionally this refactoring will also soon be used to prepare different data
structures for operation overloadings, avoiding the need to walk those ASTs
twice.

crates/webidl-tests/simple.js

@@ -142,3 +142,7 @@ global.MixinFoo = class MixinFoo {
     this._bar += other;
   }
 };
+
+global.Overloads = class {
+  foo() {}
+};

crates/webidl-tests/simple.rs

@@ -114,3 +114,11 @@ fn mixin() {
     f.add_to_bar(MixinFoo::default_bar());
     assert_eq!(f.bar(), 8);
 }
+
+#[wasm_bindgen_test]
+fn overload_naming() {
+    let o = Overloads::new().unwrap();
+    o.foo();
+    o.foo_with_arg("x");
+    o.foo_with_arg_and_a("x", 3);
+}

crates/webidl-tests/simple.webidl

@@ -88,3 +88,10 @@ partial interface mixin MixinBar {
 };
 
 MixinFoo includes MixinBar;
+
+[Constructor()]
+interface Overloads {
+  void foo();
+  void foo(DOMString arg, optional long a);
+  void foo(DOMString arg, (float or short) b);
+};

crates/webidl/src/first_pass.rs

@@ -11,10 +11,10 @@ use std::collections::{BTreeMap, BTreeSet};
 
 use weedle::{DictionaryDefinition, PartialDictionaryDefinition};
 use weedle::argument::Argument;
-use weedle::attribute::ExtendedAttribute;
+use weedle::attribute::*;
-use weedle::interface::{StringifierOrStatic, Special};
+use weedle::interface::*;
-use weedle::mixin::MixinMember;
+use weedle::mixin::*;
-use weedle::namespace::NamespaceMember;
+use weedle::namespace::OperationNamespaceMember;
 use weedle;
 
 use super::Result;
@@ -25,7 +25,7 @@ use util::camel_case_ident;
 #[derive(Default)]
 pub(crate) struct FirstPassRecord<'src> {
     pub(crate) interfaces: BTreeMap<&'src str, InterfaceData<'src>>,
-    pub(crate) enums: BTreeSet<&'src str>,
+    pub(crate) enums: BTreeMap<&'src str, &'src weedle::EnumDefinition<'src>>,
     /// The mixins, mapping their name to the webidl ast node for the mixin.
     pub(crate) mixins: BTreeMap<&'src str, MixinData<'src>>,
     pub(crate) typedefs: BTreeMap<&'src str, &'src weedle::types::Type<'src>>,
@@ -40,8 +40,13 @@ pub(crate) struct InterfaceData<'src> {
     /// Whether only partial interfaces were encountered
     pub(crate) partial: bool,
     pub(crate) global: bool,
+    pub(crate) attributes: Vec<&'src AttributeInterfaceMember<'src>>,
+    pub(crate) consts: Vec<&'src ConstMember<'src>>,
+    pub(crate) methods: Vec<&'src OperationInterfaceMember<'src>>,
     pub(crate) operations: BTreeMap<OperationId<'src>, OperationData<'src>>,
     pub(crate) superclass: Option<&'src str>,
+    pub(crate) definition_attributes: Option<&'src [ExtendedAttribute<'src>]>,
+    pub(crate) constructors: Vec<(&'src str, &'src [Argument<'src>])>,
 }
 
 /// We need to collect mixin data during the first pass, to be used later.
@@ -49,7 +54,9 @@ pub(crate) struct InterfaceData<'src> {
 pub(crate) struct MixinData<'src> {
     /// Whether only partial mixins were encountered
     pub(crate) partial: bool,
-    pub(crate) members: Vec<&'src MixinMember<'src>>,
+    pub(crate) attributes: Vec<&'src AttributeMixinMember<'src>>,
+    pub(crate) consts: Vec<&'src ConstMember<'src>>,
+    pub(crate) methods: Vec<&'src OperationMixinMember<'src>>,
     pub(crate) operations: BTreeMap<OperationId<'src>, OperationData<'src>>,
 }
 
@@ -57,8 +64,7 @@ pub(crate) struct MixinData<'src> {
 #[derive(Default)]
 pub(crate) struct NamespaceData<'src> {
     /// Whether only partial namespaces were encountered
-    pub(crate) partial: bool,
+    pub(crate) members: Vec<&'src OperationNamespaceMember<'src>>,
-    pub(crate) members: Vec<&'src NamespaceMember<'src>>,
     pub(crate) operations: BTreeMap<OperationId<'src>, OperationData<'src>>,
 }
 
@@ -117,8 +123,14 @@ impl<'src> FirstPass<'src, ()> for weedle::Definition<'src> {
             Namespace(namespace) => namespace.first_pass(record, ()),
             PartialNamespace(namespace) => namespace.first_pass(record, ()),
             Typedef(typedef) => typedef.first_pass(record, ()),
-            _ => {
+
-                // Other definitions aren't currently used in the first pass
+            Implements(_) => Ok(()),
+            Callback(..) => {
+                warn!("Unsupported WebIDL Callback definition: {:?}", self);
+                Ok(())
+            }
+            CallbackInterface(..) => {
+                warn!("Unsupported WebIDL CallbackInterface definition: {:?}", self);
                 Ok(())
             }
         }
@@ -127,6 +139,10 @@ impl<'src> FirstPass<'src, ()> for weedle::Definition<'src> {
 
 impl<'src> FirstPass<'src, ()> for weedle::DictionaryDefinition<'src> {
     fn first_pass(&'src self, record: &mut FirstPassRecord<'src>, (): ()) -> Result<()> {
+        if util::is_chrome_only(&self.attributes) {
+            return Ok(());
+        }
+
         record.dictionaries.entry(self.identifier.0)
             .or_default()
             .definition = Some(self);
@@ -136,6 +152,10 @@ impl<'src> FirstPass<'src, ()> for weedle::DictionaryDefinition<'src> {
 
 impl<'src> FirstPass<'src, ()> for weedle::PartialDictionaryDefinition<'src> {
     fn first_pass(&'src self, record: &mut FirstPassRecord<'src>, (): ()) -> Result<()> {
+        if util::is_chrome_only(&self.attributes) {
+            return Ok(());
+        }
+
         record.dictionaries.entry(self.identifier.0)
             .or_default()
             .partials
@@ -150,7 +170,7 @@ impl<'src> FirstPass<'src, ()> for weedle::EnumDefinition<'src> {
             return Ok(());
         }
 
-        if !record.enums.insert(self.identifier.0) {
+        if record.enums.insert(self.identifier.0, self).is_some() {
             info!("Encountered multiple enum declarations: {}", self.identifier.0);
         }
 
@@ -187,7 +207,7 @@ fn first_pass_operation<'src>(
     self_name: &'src str,
     ids: &[OperationId<'src>],
     arguments: &[Argument<'src>],
-) -> Result<()> {
+) {
     let mut names = Vec::with_capacity(arguments.len());
     for argument in arguments {
         match argument {
@@ -228,8 +248,6 @@ fn first_pass_operation<'src>(
             .and_modify(|same_argument_names| *same_argument_names = true)
             .or_insert(false);
     }
-
-    Ok(())
 }
 
 impl<'src> FirstPass<'src, ()> for weedle::InterfaceDefinition<'src> {
@@ -238,6 +256,11 @@ impl<'src> FirstPass<'src, ()> for weedle::InterfaceDefinition<'src> {
             return Ok(());
         }
 
+        if util::is_no_interface_object(&self.attributes) {
+            info!("Skipping because of `NoInterfaceObject` attribute: {:?}", self.identifier.0);
+            return Ok(());
+        }
+
         {
             let interface_data = record
                 .interfaces
@@ -245,11 +268,16 @@ impl<'src> FirstPass<'src, ()> for weedle::InterfaceDefinition<'src> {
                 .or_default();
             interface_data.partial = false;
             interface_data.superclass = self.inheritance.map(|s| s.identifier.0);
+            interface_data.definition_attributes = self.attributes.as_ref()
+                .map(|l| &l.body.list[..]);
         }
-
         if let Some(attrs) = &self.attributes {
-            for attr in &attrs.body.list {
+            for attr in attrs.body.list.iter() {
-                attr.first_pass(record, self.identifier.0)?;
+                process_interface_attribute(
+                    record,
+                    self.identifier.0,
+                    attr,
+                );
             }
         }
 
@@ -261,84 +289,123 @@ impl<'src> FirstPass<'src, ()> for weedle::InterfaceDefinition<'src> {
     }
 }
 
+fn process_interface_attribute<'src>(
+    record: &mut FirstPassRecord<'src>,
+    self_name: &'src str,
+    attr: &'src ExtendedAttribute<'src>
+) {
+    match attr {
+        ExtendedAttribute::ArgList(list)
+            if list.identifier.0 == "Constructor" =>
+        {
+            record.interfaces
+                .get_mut(self_name)
+                .unwrap()
+                .constructors.push((self_name, &list.args.body.list));
+            first_pass_operation(
+                record,
+                FirstPassOperationType::Interface,
+                self_name,
+                &[OperationId::Constructor],
+                &list.args.body.list,
+            )
+        }
+        ExtendedAttribute::NoArgs(other) if (other.0).0 == "Constructor" => {
+            record.interfaces
+                .get_mut(self_name)
+                .unwrap()
+                .constructors.push((self_name, &[]));
+            first_pass_operation(
+                record,
+                FirstPassOperationType::Interface,
+                self_name,
+                &[OperationId::Constructor],
+                &[],
+            )
+        }
+        ExtendedAttribute::NamedArgList(list)
+            if list.lhs_identifier.0 == "NamedConstructor" =>
+        {
+            record.interfaces
+                .get_mut(self_name)
+                .unwrap()
+                .constructors.push((list.rhs_identifier.0, &list.args.body.list));
+            first_pass_operation(
+                record,
+                FirstPassOperationType::Interface,
+                self_name,
+                &[OperationId::Constructor],
+                &list.args.body.list,
+            )
+        }
+        ExtendedAttribute::Ident(id) if id.lhs_identifier.0 == "Global" => {
+            record.interfaces.get_mut(self_name).unwrap().global = true;
+        }
+        ExtendedAttribute::IdentList(id) if id.identifier.0 == "Global" => {
+            record.interfaces.get_mut(self_name).unwrap().global = true;
+        }
+        _ => {}
+    }
+}
+
 impl<'src> FirstPass<'src, ()> for weedle::PartialInterfaceDefinition<'src> {
     fn first_pass(&'src self, record: &mut FirstPassRecord<'src>, (): ()) -> Result<()> {
         if util::is_chrome_only(&self.attributes) {
             return Ok(());
         }
-
         record
             .interfaces
             .entry(self.identifier.0)
             .or_insert_with(||
                 InterfaceData {
                     partial: true,
-                    global: false,
+                    ..Default::default()
-                    operations: Default::default(),
-                    superclass: None,
                 },
             );
-
         for member in &self.members.body {
             member.first_pass(record, self.identifier.0)?;
         }
-
         Ok(())
     }
 }
 
-impl<'src> FirstPass<'src, &'src str> for ExtendedAttribute<'src> {
-    fn first_pass(&'src self, record: &mut FirstPassRecord<'src>, self_name: &'src str) -> Result<()> {
-        match self {
-            ExtendedAttribute::ArgList(list) if list.identifier.0 == "Constructor" => {
-                first_pass_operation(
-                    record,
-                    FirstPassOperationType::Interface,
-                    self_name,
-                    &[OperationId::Constructor],
-                    &list.args.body.list,
-                )
-            }
-            ExtendedAttribute::NoArgs(name) if (name.0).0 == "Constructor" => {
-                first_pass_operation(
-                    record,
-                    FirstPassOperationType::Interface,
-                    self_name,
-                    &[OperationId::Constructor],
-                    &[],
-                )
-            }
-            ExtendedAttribute::NamedArgList(list)
-                if list.lhs_identifier.0 == "NamedConstructor" =>
-            {
-                first_pass_operation(
-                    record,
-                    FirstPassOperationType::Interface,
-                    self_name,
-                    &[OperationId::Constructor],
-                    &list.args.body.list,
-                )
-            }
-            ExtendedAttribute::Ident(id) if id.lhs_identifier.0 == "Global" => {
-                record.interfaces.get_mut(self_name).unwrap().global = true;
-                Ok(())
-            }
-            ExtendedAttribute::IdentList(id) if id.identifier.0 == "Global" => {
-                record.interfaces.get_mut(self_name).unwrap().global = true;
-                Ok(())
-            }
-            _ => Ok(())
-        }
-    }
-}
-
 impl<'src> FirstPass<'src, &'src str> for weedle::interface::InterfaceMember<'src> {
     fn first_pass(&'src self, record: &mut FirstPassRecord<'src>, self_name: &'src str) -> Result<()> {
         match self {
-            weedle::interface::InterfaceMember::Operation(op) => {
+            InterfaceMember::Attribute(attr) => {
+                attr.first_pass(record, self_name)
+            }
+            InterfaceMember::Operation(op) => {
                 op.first_pass(record, self_name)
             }
-            _ => Ok(()),
+            InterfaceMember::Const(const_) => {
+                if util::is_chrome_only(&const_.attributes) {
+                    return Ok(());
+                }
+                record.interfaces
+                    .get_mut(self_name)
+                    .unwrap()
+                    .consts
+                    .push(const_);
+                Ok(())
+            }
+            InterfaceMember::Iterable(_iterable) => {
+                warn!("Unsupported WebIDL iterable interface member: {:?}", self);
+                Ok(())
+            }
+            // TODO
+            InterfaceMember::Maplike(_) => {
+                warn!("Unsupported WebIDL Maplike interface member: {:?}", self);
+                Ok(())
+            }
+            InterfaceMember::Stringifier(_) => {
+                warn!("Unsupported WebIDL Stringifier interface member: {:?}", self);
+                Ok(())
+            }
+            InterfaceMember::Setlike(_) => {
+                warn!("Unsupported WebIDL Setlike interface member: {:?}", self);
+                Ok(())
+            }
         }
     }
 }
@@ -357,6 +424,13 @@ impl<'src> FirstPass<'src, &'src str> for weedle::interface::OperationInterfaceM
             warn!("Unsupported webidl stringifier: {:?}", self);
             return Ok(())
         }
+
+        record.interfaces
+            .get_mut(self_name)
+            .unwrap()
+            .methods
+            .push(self);
+
         let mut ids = vec![OperationId::Operation(self.identifier.map(|s| s.0))];
         for special in self.specials.iter() {
             ids.push(match special {
@@ -372,7 +446,23 @@ impl<'src> FirstPass<'src, &'src str> for weedle::interface::OperationInterfaceM
             self_name,
             &ids,
             &self.args.body.list,
-        )
+        );
+        Ok(())
+    }
+}
+
+impl<'src> FirstPass<'src, &'src str> for weedle::interface::AttributeInterfaceMember<'src> {
+    fn first_pass(&'src self, record: &mut FirstPassRecord<'src>, self_name: &'src str) -> Result<()> {
+        if util::is_chrome_only(&self.attributes) {
+            return Ok(());
+        }
+
+        record.interfaces
+            .get_mut(self_name)
+            .unwrap()
+            .attributes
+            .push(self);
+        Ok(())
     }
 }
 
@@ -388,7 +478,6 @@ impl<'src> FirstPass<'src, ()> for weedle::InterfaceMixinDefinition<'src>{
                 .entry(self.identifier.0)
                 .or_default();
             mixin_data.partial = false;
-            mixin_data.members.extend(&self.members.body);
         }
 
         for member in &self.members.body {
@@ -411,12 +500,9 @@ impl<'src> FirstPass<'src, ()> for weedle::PartialInterfaceMixinDefinition<'src>
             .or_insert_with(||
                 MixinData {
                     partial: true,
-                    members: Default::default(),
+                    ..Default::default()
-                    operations: Default::default(),
                 },
-            )
+            );
-            .members
-            .extend(&self.members.body);
 
         for member in &self.members.body {
             member.first_pass(record, self.identifier.0)?;
@@ -429,10 +515,23 @@ impl<'src> FirstPass<'src, ()> for weedle::PartialInterfaceMixinDefinition<'src>
 impl<'src> FirstPass<'src, &'src str> for weedle::mixin::MixinMember<'src> {
     fn first_pass(&'src self, record: &mut FirstPassRecord<'src>, self_name: &'src str) -> Result<()> {
         match self {
-            weedle::mixin::MixinMember::Operation(op) => {
+            MixinMember::Operation(op) => op.first_pass(record, self_name),
-                op.first_pass(record, self_name)
+            MixinMember::Attribute(a) => a.first_pass(record, self_name),
+            MixinMember::Const(a) => {
+                if util::is_chrome_only(&a.attributes) {
+                    return Ok(());
+                }
+                record.mixins
+                    .get_mut(self_name)
+                    .unwrap()
+                    .consts
+                    .push(a);
+                Ok(())
+            }
+            MixinMember::Stringifier(_) => {
+                warn!("Unsupported WebIDL stringifier mixin member: {:?}", self);
+                Ok(())
             }
-            _ => Ok(()),
         }
     }
 }
@@ -448,13 +547,34 @@ impl<'src> FirstPass<'src, &'src str> for weedle::mixin::OperationMixinMember<'s
             return Ok(())
         }
 
+        record.mixins
+            .get_mut(self_name)
+            .unwrap()
+            .methods
+            .push(self);
+
         first_pass_operation(
             record,
             FirstPassOperationType::Mixin,
             self_name,
             &[OperationId::Operation(self.identifier.map(|s| s.0.clone()))],
             &self.args.body.list,
-        )
+        );
+        Ok(())
+    }
+}
+
+impl<'src> FirstPass<'src, &'src str> for weedle::mixin::AttributeMixinMember<'src> {
+    fn first_pass(&'src self, record: &mut FirstPassRecord<'src>, self_name: &'src str) -> Result<()> {
+        if util::is_chrome_only(&self.attributes) {
+            return Ok(());
+        }
+        record.mixins
+            .get_mut(self_name)
+            .unwrap()
+            .attributes
+            .push(self);
+        Ok(())
     }
 }
 
@@ -481,16 +601,12 @@ impl<'src> FirstPass<'src, ()> for weedle::NamespaceDefinition<'src> {
         record
             .namespaces
             .entry(self.identifier.0)
-            .and_modify(|namespace_data| namespace_data.partial = false)
             .or_insert_with(||
                 NamespaceData {
-                    partial: true,
                     members: Default::default(),
                     operations: Default::default(),
                 },
-            )
+            );
-            .members
-            .extend(&self.members.body);
 
         for member in &self.members.body {
             member.first_pass(record, self.identifier.0)?;
@@ -509,9 +625,7 @@ impl<'src> FirstPass<'src, ()> for weedle::PartialNamespaceDefinition<'src> {
         record
             .namespaces
             .entry(self.identifier.0)
-            .or_default()
+            .or_default();
-            .members
-            .extend(&self.members.body);
 
         for member in &self.members.body {
             member.first_pass(record, self.identifier.0)?;
@@ -538,13 +652,16 @@ impl<'src> FirstPass<'src, &'src str> for weedle::namespace::OperationNamespaceM
             return Ok(())
         }
 
+        record.namespaces.get_mut(self_name).unwrap().members.push(self);
+
         first_pass_operation(
             record,
             FirstPassOperationType::Namespace,
             self_name,
             &[OperationId::Operation(self.identifier.map(|s| s.0.clone()))],
             &self.args.body.list,
-        )
+        );
+        Ok(())
     }
 }
 

crates/webidl/src/idl_type.rs

@@ -287,7 +287,7 @@ impl<'a> ToIdlType<'a> for Identifier<'a> {
             Some(IdlType::Interface(self.0))
         } else if record.dictionaries.contains_key(self.0) {
             Some(IdlType::Dictionary(self.0))
-        } else if record.enums.contains(self.0) {
+        } else if record.enums.contains_key(self.0) {
             Some(IdlType::Enum(self.0))
         } else {
             warn!("Unrecognized type: {}", self.0);

examples/julia_set/Cargo.toml

@@ -8,3 +8,4 @@ crate-type = ["cdylib"]
 
 [dependencies]
 wasm-bindgen = { path = "../.." }
+web-sys = { path = "../../crates/web-sys" }

examples/julia_set/src/lib.rs

@@ -1,18 +1,13 @@
 extern crate wasm_bindgen;
+extern crate web_sys;
+
 use wasm_bindgen::prelude::*;
+use web_sys::{CanvasRenderingContext2D, ImageData};
 
 mod fractal;
 use fractal::get_julia_set;
 use fractal::complex::Complex;
 
-#[wasm_bindgen]
-extern "C" {
-    pub type ImageData;
-
-    #[wasm_bindgen(constructor)]
-    pub fn new(arr: &Uint8ClampedArray, width: u32, height: u32) -> ImageData;
-}
-
 #[wasm_bindgen]
 extern "C" {
     pub type Uint8ClampedArray;
@@ -21,19 +16,13 @@ extern "C" {
     pub fn new(arr: &[u8]) -> Uint8ClampedArray;
 }
 
-#[wasm_bindgen]
-extern "C" {
-    pub type CanvasRenderingContext2D;
-
-    #[wasm_bindgen(method, js_name = putImageData)]
-    pub fn put_image_data(this: &CanvasRenderingContext2D, image_data: &ImageData, p_1: i32, p_2: i32);
-}
-
 #[wasm_bindgen]
 pub fn draw(ctx: &CanvasRenderingContext2D, width: u32, height: u32, real: f64, imaginary: f64) {
     let c = Complex { real, imaginary };
     let data = get_julia_set(width, height, c);
     let uint8_array = Uint8ClampedArray::new(&data);
 
-    ctx.put_image_data(&ImageData::new(&uint8_array, width, height), 0, 0);
+    ctx.put_image_data(
+        &ImageData::with_data_and_sw_and_sh_with_sh(&data, width, height).unwrap(),
+        0.0, 0.0);
 }