Add a new style of test suite for the witx tooling similar to *.wast (#392)

* Add a wast-like testsuite for the witx tool

As the functionality of `witx` grows this will hopefully make adding
tests for new functionality as well as new kinds of tests easier. The
goal is to make it very easy to drop a test file with various directives
to exercise the functionality of `witx` and its internals.

For now the testsuite is quite simple, simply asserting whether
documents are either valid or invalid. My hope, though, is that this can
be expanded over time with more styles of assertions directives.

* Migrate union test to `union.witxt`

* Add documentation/roundtrip testing to all documents

* Translate multi-module test suite to `*.witxt`

* Move wasi tests into `witxt` test suites

* Convert representation tests to `*.witxt`

* Rebased onto origin/main

Author: alexcrichton

phases/ephemeral/docs.md

@@ -871,12 +871,12 @@ Alignment: 8
 - align: 8
 - tag_size: 1
 ### Variant cases
+- <a href="#event_u.clock" name="event_u.clock"></a> `clock`
+
 - <a href="#event_u.fd_read" name="event_u.fd_read"></a> `fd_read`: [`event_fd_readwrite`](#event_fd_readwrite)
 
 - <a href="#event_u.fd_write" name="event_u.fd_write"></a> `fd_write`: [`event_fd_readwrite`](#event_fd_readwrite)
 
-- <a href="#event_u.clock" name="event_u.clock"></a> `clock`
-
 ## <a href="#event" name="event"></a> `event`: Record
 An event that occurred.
 

tools/witx/Cargo.toml

@@ -23,3 +23,8 @@ wast = { version = "22.0.0", default-features = false }
 diff = "0.1.11"
 pretty_env_logger = "0.4"
 structopt = "0.3"
+rayon = "1.0"
+
+[[test]]
+name = "witxt"
+harness = false

tools/witx/src/lib.rs

@@ -9,7 +9,7 @@ mod io;
 /// Calculate memory layout of types
 mod layout;
 /// Witx syntax parsing from SExprs
-mod parser;
+pub mod parser;
 /// Paths to witx documents for various proposal phases
 pub mod phases;
 /// Calculate required polyfill between interfaces
@@ -28,10 +28,9 @@ pub use coretypes::{AtomType, CoreFuncType, CoreParamSignifies, CoreParamType, T
 pub use docs::Documentation;
 pub use io::{Filesystem, MockFs, WitxIo};
 pub use layout::{Layout, RecordMemberLayout, SizeAlign};
-pub use parser::DeclSyntax;
 pub use render::SExpr;
 pub use representation::{RepEquality, Representable};
-pub use validate::ValidationError;
+pub use validate::{DocValidation, ValidationError};
 
 use std::path::{Path, PathBuf};
 use thiserror::Error;

tools/witx/src/parser.rs

@@ -477,7 +477,6 @@ impl<'a> Parse<'a> for UnionSyntax<'a> {
             None
         };
         let mut fields = Vec::new();
-        fields.push(parser.parse()?);
         while !parser.is_empty() {
             fields.push(parser.parse()?);
         }
@@ -505,7 +504,9 @@ impl<'a> Parse<'a> for VariantSyntax<'a> {
         };
         let mut cases = Vec::new();
         while !parser.is_empty() {
-            cases.push(parser.parens(|p| p.parse())?);
+            let comments = parser.parse()?;
+            let item = parser.parens(|p| p.parse())?;
+            cases.push(Documented { comments, item });
         }
         Ok(VariantSyntax { tag, cases })
     }

tools/witx/src/representation.rs

@@ -81,12 +81,14 @@ impl Representable for Variant {
         let other_by_name = by
             .cases
             .iter()
-            .map(|c| (&c.name, c))
+            .enumerate()
+            .map(|(i, c)| (&c.name, (c, i)))
             .collect::<HashMap<_, _>>();
         // For each variant in self, must have variant of same name in by:
-        for v in self.cases.iter() {
+        for (i, v) in self.cases.iter().enumerate() {
             let other_ty = match other_by_name.get(&v.name) {
-                Some(other) => &other.tref,
+                Some((_, j)) if i != *j => return RepEquality::NotEq,
+                Some((other, _)) => &other.tref,
                 None => return RepEquality::NotEq,
             };
             match (&v.tref, other_ty) {
@@ -157,76 +159,3 @@ impl Representable for Type {
         }
     }
 }
-
-#[cfg(test)]
-mod test {
-    use super::*;
-    use crate::io::MockFs;
-    use crate::toplevel::parse_witx_with;
-    use crate::Id;
-    use std::rc::Rc;
-
-    fn def_type(typename: &str, syntax: &str) -> Rc<NamedType> {
-        use std::path::Path;
-        let doc = parse_witx_with(&[Path::new("-")], &MockFs::new(&[("-", syntax)]))
-            .expect("parse witx doc");
-        let t = doc.typename(&Id::new(typename)).expect("defined type");
-        // Identity should always be true:
-        assert_eq!(t.representable(&t), RepEquality::Eq, "identity");
-        t
-    }
-
-    #[test]
-    fn different_typenames() {
-        let a = def_type("a", "(typename $a (flags (@witx bitflags u32) $b $c))");
-        let d = def_type("d", "(typename $d (flags (@witx bitflags u32) $b $c))");
-
-        assert_eq!(a.representable(&d), RepEquality::Eq);
-        assert_eq!(d.representable(&a), RepEquality::Eq);
-    }
-
-    #[test]
-    fn enum_() {
-        let base = def_type("a", "(typename $a (enum $b $c))");
-        let extra_variant = def_type("a", "(typename $a (enum $b $c $d))");
-
-        assert_eq!(base.representable(&extra_variant), RepEquality::Superset);
-        assert_eq!(extra_variant.representable(&base), RepEquality::NotEq);
-
-        let smaller_size = def_type("a", "(typename $a (enum (@witx tag u16) $b $c))");
-        assert_eq!(smaller_size.representable(&base), RepEquality::Superset);
-        assert_eq!(
-            smaller_size.representable(&extra_variant),
-            RepEquality::Superset
-        );
-    }
-
-    #[test]
-    fn union() {
-        let base = def_type(
-            "a",
-            "(typename $tag (enum (@witx tag u8) $b $c))
-            (typename $a (union (@witx tag $tag) u32 f32))",
-        );
-        let extra_variant = def_type(
-            "a",
-            "(typename $tag (enum (@witx tag u8) $b $c $d))
-            (typename $a (union (@witx tag $tag) u32 f32 f64))",
-        );
-
-        assert_eq!(base.representable(&extra_variant), RepEquality::Superset);
-        assert_eq!(extra_variant.representable(&base), RepEquality::NotEq);
-
-        let other_ordering = def_type(
-            "a",
-            "(typename $tag (enum (@witx tag u8) $b $c))
-            (typename $a (variant (@witx tag $tag) (case $c f32) (case $b u32)))",
-        );
-        assert_eq!(base.representable(&other_ordering), RepEquality::Eq);
-        assert_eq!(other_ordering.representable(&base), RepEquality::Eq);
-        assert_eq!(
-            other_ordering.representable(&extra_variant),
-            RepEquality::Superset
-        );
-    }
-}

tools/witx/src/toplevel.rs

@@ -32,7 +32,7 @@ fn _parse_witx_with(paths: &[&Path], io: &dyn WitxIo) -> Result<Document, WitxEr
             &mut parsed,
         )?;
     }
-    Ok(Document::new(definitions, validator.entries))
+    Ok(validator.into_document(definitions))
 }
 
 fn parse_file(

tools/witx/src/validate.rs

@@ -5,10 +5,10 @@ use crate::{
         ImportTypeSyntax, ModuleDeclSyntax, RecordSyntax, TypedefSyntax, UnionSyntax,
         VariantSyntax,
     },
-    BuiltinType, Case, Constant, Definition, Entry, HandleDatatype, Id, IntRepr, InterfaceFunc,
-    InterfaceFuncParam, InterfaceFuncParamPosition, Location, Module, ModuleDefinition,
-    ModuleEntry, ModuleImport, ModuleImportVariant, NamedType, RecordDatatype, RecordMember, Type,
-    TypePassedBy, TypeRef, Variant,
+    BuiltinType, Case, Constant, Definition, Document, Entry, HandleDatatype, Id, IntRepr,
+    InterfaceFunc, InterfaceFuncParam, InterfaceFuncParamPosition, Location, Module,
+    ModuleDefinition, ModuleEntry, ModuleImport, ModuleImportVariant, NamedType, RecordDatatype,
+    RecordMember, Type, TypePassedBy, TypeRef, Variant,
 };
 use std::collections::{HashMap, HashSet};
 use std::path::Path;
@@ -129,7 +129,7 @@ impl IdentValidation {
 
 pub struct DocValidation {
     scope: IdentValidation,
-    pub entries: HashMap<Id, Entry>,
+    entries: HashMap<Id, Entry>,
     constant_scopes: HashMap<Id, IdentValidation>,
 }
 
@@ -155,6 +155,10 @@ impl DocValidation {
             path,
         }
     }
+
+    pub fn into_document(self, defs: Vec<Definition>) -> Document {
+        Document::new(defs, self.entries)
+    }
 }
 
 impl DocValidationScope<'_> {
@@ -439,14 +443,16 @@ impl DocValidationScope<'_> {
             }
         }
 
-        let mut names = names.map(|names| names.into_iter().collect::<HashSet<_>>());
+        let mut name_set = names
+            .as_ref()
+            .map(|names| names.iter().collect::<HashSet<_>>());
 
-        let cases = syntax
+        let mut cases = syntax
             .cases
             .iter()
             .map(|case| {
                 let name = Id::new(case.item.name.name());
-                if let Some(names) = &mut names {
+                if let Some(names) = &mut name_set {
                     if !names.remove(&name) {
                         return Err(ValidationError::InvalidUnionField {
                             name: name.as_str().to_string(),
@@ -467,6 +473,18 @@ impl DocValidationScope<'_> {
                 })
             })
             .collect::<Result<Vec<_>, _>>()?;
+
+        // If we have an explicit tag with an enum then that's instructing us to
+        // reorder cases based on the order of the enum itself, so do that here.
+        if let Some(names) = names {
+            let name_pos = names
+                .iter()
+                .enumerate()
+                .map(|(i, name)| (name, i))
+                .collect::<HashMap<_, _>>();
+            cases.sort_by_key(|c| name_pos[&&c.name]);
+        }
+
         Ok(Variant { tag_repr, cases })
     }