From 36fcffcc6e685674c9c898c27ce9adfb6ad4c17b Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Wed, 10 Feb 2021 08:52:31 -0800
Subject: [PATCH 1/7] 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.
---
 tools/witx/Cargo.toml                  |   5 +
 tools/witx/src/lib.rs                  |   5 +-
 tools/witx/src/toplevel.rs             |   2 +-
 tools/witx/src/validate.rs             |   6 +-
 tools/witx/tests/anonymous.rs          |  43 -----
 tools/witx/tests/witxt.rs              | 243 +++++++++++++++++++++++++
 tools/witx/tests/witxt/anonymous.witxt |  56 ++++++
 tools/witx/tests/witxt/simple.witxt    |  12 ++
 8 files changed, 324 insertions(+), 48 deletions(-)
 delete mode 100644 tools/witx/tests/anonymous.rs
 create mode 100644 tools/witx/tests/witxt.rs
 create mode 100644 tools/witx/tests/witxt/anonymous.witxt
 create mode 100644 tools/witx/tests/witxt/simple.witxt

diff --git a/tools/witx/Cargo.toml b/tools/witx/Cargo.toml
index 34c074d68..bf9ce16cd 100644
--- a/tools/witx/Cargo.toml
+++ b/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
diff --git a/tools/witx/src/lib.rs b/tools/witx/src/lib.rs
index 9c0f53060..2c876aa13 100644
--- a/tools/witx/src/lib.rs
+++ b/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;
diff --git a/tools/witx/src/toplevel.rs b/tools/witx/src/toplevel.rs
index 993fc8cc4..83e940c7c 100644
--- a/tools/witx/src/toplevel.rs
+++ b/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(
diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs
index 88ae10147..7460d8e76 100644
--- a/tools/witx/src/validate.rs
+++ b/tools/witx/src/validate.rs
@@ -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<'_> {
diff --git a/tools/witx/tests/anonymous.rs b/tools/witx/tests/anonymous.rs
deleted file mode 100644
index 6cfb2ea7e..000000000
--- a/tools/witx/tests/anonymous.rs
+++ /dev/null
@@ -1,43 +0,0 @@
-use witx;
-
-fn is_anonymous_record_err(r: Result<witx::Document, witx::WitxError>) -> bool {
-    match r {
-        Err(witx::WitxError::Validation(witx::ValidationError::AnonymousRecord { .. })) => true,
-        _ => false,
-    }
-}
-
-#[test]
-fn anonymous_types() {
-    let pointer_to_record = witx::parse("(typename $a (@witx pointer (record (field $b u8))))");
-    assert!(is_anonymous_record_err(pointer_to_record));
-
-    let pointer_to_union =
-        witx::parse("(typename $tag (enum $b)) (typename $a (@witx pointer (union u8)))");
-    assert!(is_anonymous_record_err(pointer_to_union));
-
-    let pointer_to_enum = witx::parse("(typename $a (@witx pointer (enum $b)))");
-    assert!(is_anonymous_record_err(pointer_to_enum));
-
-    let pointer_to_flags = witx::parse("(typename $a (@witx pointer (flags $b)))");
-    assert!(is_anonymous_record_err(pointer_to_flags));
-
-    let pointer_to_handle = witx::parse("(typename $a (@witx pointer (handle)))");
-    assert!(is_anonymous_record_err(pointer_to_handle));
-
-    let pointer_to_builtin = witx::parse("(typename $a (@witx pointer u8))");
-    assert!(pointer_to_builtin.is_ok());
-
-    let pointer_to_pointer = witx::parse("(typename $a (@witx pointer (@witx const_pointer u8)))");
-    assert!(pointer_to_pointer.is_ok());
-
-    let record_in_record = witx::parse("(typename $a (record (field $b (record (field $c u8)))))");
-    assert!(is_anonymous_record_err(record_in_record));
-
-    let union_in_record =
-        witx::parse("(typename $tag (enum $c)) (typename $a (record (field $b (union u8))))");
-    assert!(is_anonymous_record_err(union_in_record));
-
-    let pointer_in_record = witx::parse("(typename $a (record (field $b (@witx pointer u8))))");
-    assert!(pointer_in_record.is_ok())
-}
diff --git a/tools/witx/tests/witxt.rs b/tools/witx/tests/witxt.rs
new file mode 100644
index 000000000..07e72827a
--- /dev/null
+++ b/tools/witx/tests/witxt.rs
@@ -0,0 +1,243 @@
+//! You can run this test suite with:
+//!
+//!     cargo test --test witxt
+//!
+//! An argument can be passed as well to filter, based on filename, which test
+//! to run
+//!
+//!     cargo test --test witxt foo.witxt
+
+use anyhow::{bail, Context, Result};
+use rayon::prelude::*;
+use std::path::{Path, PathBuf};
+use std::str;
+use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
+use wast::parser::{self, Parse, ParseBuffer, Parser};
+
+fn main() {
+    let tests = find_tests();
+    let filter = std::env::args().nth(1);
+
+    let tests = tests
+        .par_iter()
+        .filter_map(|test| {
+            if let Some(filter) = &filter {
+                if let Some(s) = test.to_str() {
+                    if !s.contains(filter) {
+                        return None;
+                    }
+                }
+            }
+            let contents = std::fs::read(test).unwrap();
+            Some((test, contents))
+        })
+        .collect::<Vec<_>>();
+
+    println!("running {} test files\n", tests.len());
+
+    let state = TestState::default();
+    let errors = tests
+        .par_iter()
+        .filter_map(|(test, contents)| state.run_test(test, contents).err())
+        .collect::<Vec<_>>();
+
+    if !errors.is_empty() {
+        for msg in errors.iter() {
+            eprintln!("{:?}", msg);
+        }
+
+        panic!("{} tests failed", errors.len())
+    }
+
+    println!(
+        "test result: ok. {} directives passed\n",
+        state.ntests.load(SeqCst)
+    );
+}
+
+/// Recursively finds all tests in a whitelisted set of directories which we
+/// then load up and test in parallel.
+fn find_tests() -> Vec<PathBuf> {
+    let mut tests = Vec::new();
+    find_tests("tests/witxt".as_ref(), &mut tests);
+    tests.sort();
+    return tests;
+
+    fn find_tests(path: &Path, tests: &mut Vec<PathBuf>) {
+        for f in path.read_dir().unwrap() {
+            let f = f.unwrap();
+            if f.file_type().unwrap().is_dir() {
+                find_tests(&f.path(), tests);
+                continue;
+            }
+
+            match f.path().extension().and_then(|s| s.to_str()) {
+                Some("witxt") => {}
+                _ => continue,
+            }
+            tests.push(f.path());
+        }
+    }
+}
+
+#[derive(Default)]
+struct TestState {
+    ntests: AtomicUsize,
+}
+
+impl TestState {
+    fn run_test(&self, test: &Path, contents: &[u8]) -> Result<()> {
+        let contents = str::from_utf8(contents)?;
+        macro_rules! adjust {
+            ($e:expr) => {{
+                let mut e = wast::Error::from($e);
+                e.set_path(test);
+                e.set_text(contents);
+                e
+            }};
+        }
+        let buf = ParseBuffer::new(contents).map_err(|e| adjust!(e))?;
+        let witxt = parser::parse::<Witxt>(&buf).map_err(|e| adjust!(e))?;
+
+        let errors = witxt
+            .directives
+            .into_par_iter()
+            .filter_map(|directive| {
+                let (line, col) = directive.span().linecol_in(contents);
+                self.test_directive(contents, test, directive)
+                    .with_context(|| {
+                        format!(
+                            "failed directive on {}:{}:{}",
+                            test.display(),
+                            line + 1,
+                            col + 1
+                        )
+                    })
+                    .err()
+            })
+            .collect::<Vec<_>>();
+        if errors.is_empty() {
+            return Ok(());
+        }
+        let mut s = format!("{} test failures in {}:", errors.len(), test.display());
+        for mut error in errors {
+            if let Some(err) = error.downcast_mut::<wast::Error>() {
+                err.set_path(test);
+                err.set_text(contents);
+            }
+            s.push_str("\n\n\t--------------------------------\n\n\t");
+            s.push_str(&format!("{:?}", error).replace("\n", "\n\t"));
+        }
+        bail!("{}", s)
+    }
+
+    fn test_directive(&self, contents: &str, test: &Path, directive: WitxtDirective) -> Result<()> {
+        match directive {
+            WitxtDirective::Witx(witx) => {
+                witx.document(contents, test)?;
+                self.bump_ntests();
+            }
+            WitxtDirective::AssertInvalid { witx, message, .. } => {
+                let err = match witx.document(contents, test) {
+                    Ok(_) => bail!("witx was valid when it shouldn't be"),
+                    Err(e) => format!("{:?}", anyhow::Error::from(e)),
+                };
+                if !err.contains(message) {
+                    bail!("expected error {:?}\nfound error {}", message, err);
+                }
+                self.bump_ntests();
+            }
+        }
+        Ok(())
+    }
+
+    fn bump_ntests(&self) {
+        self.ntests.fetch_add(1, SeqCst);
+    }
+}
+
+mod kw {
+    wast::custom_keyword!(assert_invalid);
+    wast::custom_keyword!(witx);
+}
+
+struct Witxt<'a> {
+    directives: Vec<WitxtDirective<'a>>,
+}
+
+impl<'a> Parse<'a> for Witxt<'a> {
+    fn parse(parser: Parser<'a>) -> parser::Result<Self> {
+        let mut directives = Vec::new();
+        while !parser.is_empty() {
+            directives.push(parser.parens(|p| p.parse())?);
+        }
+        Ok(Witxt { directives })
+    }
+}
+
+enum WitxtDirective<'a> {
+    Witx(Witx<'a>),
+    AssertInvalid {
+        span: wast::Span,
+        witx: Witx<'a>,
+        message: &'a str,
+    },
+}
+
+impl WitxtDirective<'_> {
+    fn span(&self) -> wast::Span {
+        match self {
+            WitxtDirective::Witx(w) => w.span,
+            WitxtDirective::AssertInvalid { span, .. } => *span,
+        }
+    }
+}
+
+impl<'a> Parse<'a> for WitxtDirective<'a> {
+    fn parse(parser: Parser<'a>) -> parser::Result<Self> {
+        let mut l = parser.lookahead1();
+        if l.peek::<kw::witx>() {
+            Ok(WitxtDirective::Witx(parser.parse()?))
+        } else if l.peek::<kw::assert_invalid>() {
+            let span = parser.parse::<kw::assert_invalid>()?.0;
+            Ok(WitxtDirective::AssertInvalid {
+                span,
+                witx: parser.parens(|p| p.parse())?,
+                message: parser.parse()?,
+            })
+        } else {
+            Err(l.error())
+        }
+    }
+}
+
+struct Witx<'a> {
+    span: wast::Span,
+    decls: Vec<witx::parser::Documented<'a, witx::parser::DeclSyntax<'a>>>,
+}
+
+impl Witx<'_> {
+    fn document(&self, contents: &str, file: &Path) -> Result<witx::Document> {
+        let mut validator = witx::DocValidation::new();
+        let mut definitions = Vec::new();
+        for decl in self.decls.iter() {
+            let def = validator
+                .scope(contents, file)
+                .validate_decl(&decl.item, &decl.comments)
+                .map_err(witx::WitxError::Validation)?;
+            definitions.push(def);
+        }
+        Ok(validator.into_document(definitions))
+    }
+}
+
+impl<'a> Parse<'a> for Witx<'a> {
+    fn parse(parser: Parser<'a>) -> parser::Result<Self> {
+        let span = parser.parse::<kw::witx>()?.0;
+        let mut decls = Vec::new();
+        while !parser.is_empty() {
+            decls.push(parser.parens(|p| p.parse())?);
+        }
+        Ok(Witx { span, decls })
+    }
+}
diff --git a/tools/witx/tests/witxt/anonymous.witxt b/tools/witx/tests/witxt/anonymous.witxt
new file mode 100644
index 000000000..d7a36c12f
--- /dev/null
+++ b/tools/witx/tests/witxt/anonymous.witxt
@@ -0,0 +1,56 @@
+;; Ensure that anonymous structured types are not allowed in type positions at
+;; this time, everything has to be named to assist in binding in languages.
+
+(assert_invalid
+  (witx
+    (typename $a (@witx pointer (struct (field $b u8))))
+  )
+  "Anonymous structured types")
+
+(assert_invalid
+  (witx
+    (typename $tag (enum u8 $b))
+    (typename $a (@witx pointer (union $tag (field $b u8))))
+  )
+  "Anonymous structured types")
+
+(assert_invalid
+  (witx
+    (typename $a (@witx pointer (enum u32 $b)))
+  )
+  "Anonymous structured types")
+
+(assert_invalid
+  (witx
+    (typename $a (@witx pointer (flags u32 $b)))
+  )
+  "Anonymous structured types")
+
+(assert_invalid
+  (witx
+    (typename $a (@witx pointer (handle)))
+  )
+  "Anonymous structured types")
+
+(assert_invalid
+  (witx
+    (typename $a (struct (field $b (struct (field $c u8)))))
+  )
+  "Anonymous structured types")
+
+(assert_invalid
+  (witx
+    (typename $tag (enum u8 $c)) (typename $a (struct (field $b (union $tag (field $c u8)))))
+  )
+  "Anonymous structured types")
+
+
+;; pointers don't count for anonymous indirections
+(witx
+  (typename $a (@witx pointer u8)))
+
+(witx
+  (typename $a (@witx pointer (@witx const_pointer u8))))
+
+(witx
+  (typename $a (struct (field $b (@witx pointer u8)))))
diff --git a/tools/witx/tests/witxt/simple.witxt b/tools/witx/tests/witxt/simple.witxt
new file mode 100644
index 000000000..af2403043
--- /dev/null
+++ b/tools/witx/tests/witxt/simple.witxt
@@ -0,0 +1,12 @@
+(witx)
+
+(witx
+  (typename $x u32)
+)
+
+(assert_invalid
+  (witx
+    (typename $x u32)
+    (typename $x u32)
+  )
+  "Redefinition of name `x`")

From 5f42f65cbb4f0dfe0b0693690bb6a5f90b1862d8 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Wed, 10 Feb 2021 09:19:09 -0800
Subject: [PATCH 2/7] Migrate union test to `union.witxt`

---
 tools/witx/tests/union.rs          | 174 -----------------------------
 tools/witx/tests/witxt.rs          | 110 +++++++++++++++---
 tools/witx/tests/witxt/union.witxt |  96 ++++++++++++++++
 3 files changed, 193 insertions(+), 187 deletions(-)
 delete mode 100644 tools/witx/tests/union.rs
 create mode 100644 tools/witx/tests/witxt/union.witxt

diff --git a/tools/witx/tests/union.rs b/tools/witx/tests/union.rs
deleted file mode 100644
index 31ade263a..000000000
--- a/tools/witx/tests/union.rs
+++ /dev/null
@@ -1,174 +0,0 @@
-// Every worker needs a union! Time to organize
-use witx::{Id, Representable};
-
-#[test]
-fn one_variant_union() {
-    let d = witx::parse(
-        "(typename $tag (enum $c))
-         (typename $u (union (@witx tag $tag) u8))",
-    );
-    assert!(d.is_ok());
-}
-
-#[test]
-fn two_variant_union() {
-    let d1 = witx::parse(
-        "(typename $tag (enum $a $b))
-         (typename $u (union (@witx tag $tag) u8 u16))",
-    );
-    assert!(d1.is_ok(), "d1 is ok");
-
-    // Fields can come in whatever order:
-    let d2 = witx::parse(
-        "(typename $tag (enum $a $b))
-         (typename $u (variant (@witx tag $tag) (case $b u16) (case $a u8)))",
-    );
-    assert!(d2.is_ok(), "d2 is ok");
-
-    // These two unions should be represented the same:
-    let u1 = d1.unwrap().typename(&Id::new("u")).unwrap().type_();
-    let u2 = d2.unwrap().typename(&Id::new("u")).unwrap().type_();
-
-    assert_eq!(
-        u1.representable(&u2),
-        witx::RepEquality::Eq,
-        "u1 can represent u2"
-    );
-    assert_eq!(
-        u2.representable(&u1),
-        witx::RepEquality::Eq,
-        "u2 can represent u1"
-    );
-
-    // Tag order doesnt matter for validation
-    let d3 = witx::parse(
-        "(typename $tag (enum $b $a))
-         (typename $u (union (@witx tag $tag) u16 u8))",
-    );
-    assert!(d3.is_ok(), "d2 is ok");
-}
-
-#[test]
-fn empty_variant_unions() {
-    let d1 = witx::parse(
-        "(typename $tag (enum $a $b))
-         (typename $u (variant (@witx tag $tag) (case $a) (case $b u16)))",
-    );
-    assert!(d1.is_ok(), "d1 is ok");
-
-    let d2 = witx::parse(
-        "(typename $tag (enum $a $b))
-         (typename $u (variant (@witx tag $tag) (case $a) (case $b)))",
-    );
-    assert!(d2.is_ok(), "d2 is ok");
-}
-
-#[test]
-fn many_variant_unions() {
-    let d1 = witx::parse(
-        "(typename $tag (enum $a $b $c $d $e $f $g $h $i $j $k $l $m))
-         (typename $u
-           (variant (@witx tag $tag)
-            (case $a u8)
-            (case $b u16)
-            (case $c u32)
-            (case $d u64)
-            (case $e s8)
-            (case $f s16)
-            (case $g s32)
-            (case $h s64)
-            (case $i f32)
-            (case $j f64)
-            (case $k (@witx usize))
-            (case $l (@witx char8))
-            (case $m)
-           )
-         )",
-    );
-    assert!(d1.is_ok(), "d1 is ok");
-}
-
-#[test]
-fn no_tag_union() {
-    let d = witx::parse("(typename $u (union $tag (field $a u8) (field $b u16)))");
-    assert!(d.is_err());
-}
-
-#[test]
-fn wrong_kind_tag_union() {
-    let d = witx::parse(
-        "(typename $tag string)
-            (typename $u (union (@witx tag $tag) u8 u16))",
-    );
-    let (expected, got) = wrong_kind_name_err(d).expect("wrong kind of tag");
-    assert_eq!(expected, "enum or builtin");
-    assert_eq!(got, "list");
-}
-
-#[test]
-fn bad_field_unions() {
-    let d = witx::parse(
-        "(typename $tag (enum $c))
-         (typename $u (variant (@witx tag $tag) (case $b u8)))",
-    );
-    match validation_err(d) {
-        witx::ValidationError::InvalidUnionField { name, reason, .. } => {
-            assert_eq!(name, "b", "bad field name union 1");
-            assert_eq!(
-                reason, "does not correspond to variant in tag `tag`",
-                "reason union 1"
-            );
-        }
-        other => panic!("bad error: {}", other),
-    }
-
-    let d = witx::parse(
-        "(typename $tag (enum $c))
-         (typename $u (variant (@witx tag $tag) (case $c f32) (case $b u8)))",
-    );
-    match validation_err(d) {
-        witx::ValidationError::UnionSizeMismatch { .. } => {}
-        other => panic!("bad error: {}", other),
-    }
-
-    let d = witx::parse(
-        "(typename $tag (enum $c $d))
-         (typename $u (variant (@witx tag $tag) (case $c f32)))",
-    );
-    match validation_err(d) {
-        witx::ValidationError::UnionSizeMismatch { .. } => {}
-        other => panic!("bad error: {}", other),
-    }
-}
-
-fn wrong_kind_name_err(
-    r: Result<witx::Document, witx::WitxError>,
-) -> Option<(&'static str, &'static str)> {
-    match r {
-        Err(witx::WitxError::Validation(witx::ValidationError::WrongKindName {
-            expected,
-            got,
-            ..
-        })) => Some((expected, got)),
-        Err(e) => {
-            eprintln!("expected WrongKindName ValidationError, got: {:?}", e);
-            None
-        }
-        Ok(_) => {
-            eprintln!("expected WrongKindName ValidationError: Ok(witx::Document)");
-            None
-        }
-    }
-}
-
-fn validation_err(r: Result<witx::Document, witx::WitxError>) -> witx::ValidationError {
-    match r {
-        Err(witx::WitxError::Validation(e)) => e,
-        Err(e) => {
-            panic!("expected ValidationError, got: {:?}", e)
-        }
-        Ok(_) => {
-            panic!("expected ValidationError, got: Ok(witx::Document)")
-        }
-    }
-}
diff --git a/tools/witx/tests/witxt.rs b/tools/witx/tests/witxt.rs
index 07e72827a..4e448bee8 100644
--- a/tools/witx/tests/witxt.rs
+++ b/tools/witx/tests/witxt.rs
@@ -7,12 +7,14 @@
 //!
 //!     cargo test --test witxt foo.witxt
 
-use anyhow::{bail, Context, Result};
+use anyhow::{anyhow, bail, Context, Result};
 use rayon::prelude::*;
+use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 use std::str;
 use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
 use wast::parser::{self, Parse, ParseBuffer, Parser};
+use witx::Representable;
 
 fn main() {
     let tests = find_tests();
@@ -35,10 +37,17 @@ fn main() {
 
     println!("running {} test files\n", tests.len());
 
-    let state = TestState::default();
+    let ntests = AtomicUsize::new(0);
     let errors = tests
         .par_iter()
-        .filter_map(|(test, contents)| state.run_test(test, contents).err())
+        .filter_map(|(test, contents)| {
+            WitxtRunner {
+                ntests: &ntests,
+                documents: HashMap::new(),
+            }
+            .run(test, contents)
+            .err()
+        })
         .collect::<Vec<_>>();
 
     if !errors.is_empty() {
@@ -51,7 +60,7 @@ fn main() {
 
     println!(
         "test result: ok. {} directives passed\n",
-        state.ntests.load(SeqCst)
+        ntests.load(SeqCst)
     );
 }
 
@@ -80,13 +89,13 @@ fn find_tests() -> Vec<PathBuf> {
     }
 }
 
-#[derive(Default)]
-struct TestState {
-    ntests: AtomicUsize,
+struct WitxtRunner<'a> {
+    ntests: &'a AtomicUsize,
+    documents: HashMap<String, witx::Document>,
 }
 
-impl TestState {
-    fn run_test(&self, test: &Path, contents: &[u8]) -> Result<()> {
+impl WitxtRunner<'_> {
+    fn run(&mut self, test: &Path, contents: &[u8]) -> Result<()> {
         let contents = str::from_utf8(contents)?;
         macro_rules! adjust {
             ($e:expr) => {{
@@ -101,7 +110,7 @@ impl TestState {
 
         let errors = witxt
             .directives
-            .into_par_iter()
+            .into_iter()
             .filter_map(|directive| {
                 let (line, col) = directive.span().linecol_in(contents);
                 self.test_directive(contents, test, directive)
@@ -131,11 +140,19 @@ impl TestState {
         bail!("{}", s)
     }
 
-    fn test_directive(&self, contents: &str, test: &Path, directive: WitxtDirective) -> Result<()> {
+    fn test_directive(
+        &mut self,
+        contents: &str,
+        test: &Path,
+        directive: WitxtDirective,
+    ) -> Result<()> {
         match directive {
             WitxtDirective::Witx(witx) => {
-                witx.document(contents, test)?;
+                let doc = witx.document(contents, test)?;
                 self.bump_ntests();
+                if let Some(name) = witx.id {
+                    self.documents.insert(name.name().to_string(), doc);
+                }
             }
             WitxtDirective::AssertInvalid { witx, message, .. } => {
                 let err = match witx.document(contents, test) {
@@ -147,6 +164,32 @@ impl TestState {
                 }
                 self.bump_ntests();
             }
+            WitxtDirective::AssertRepEquality { repr, t1, t2, .. } => {
+                let (t1m, t1t) = t1;
+                let (t2m, t2t) = t2;
+                let t1d = self
+                    .documents
+                    .get(t1m.name())
+                    .ok_or_else(|| anyhow!("no document named {:?}", t1m.name()))?;
+                let t2d = self
+                    .documents
+                    .get(t2m.name())
+                    .ok_or_else(|| anyhow!("no document named {:?}", t2m.name()))?;
+                let t1 = t1d
+                    .typename(&witx::Id::new(t1t))
+                    .ok_or_else(|| anyhow!("no document named {:?}", t1t))?;
+                let t2 = t2d
+                    .typename(&witx::Id::new(t2t))
+                    .ok_or_else(|| anyhow!("no document named {:?}", t2t))?;
+                match (repr, t1.type_().representable(&t2.type_())) {
+                    (RepEquality::Eq, witx::RepEquality::Eq)
+                    | (RepEquality::NotEq, witx::RepEquality::NotEq) => {}
+                    (a, b) => {
+                        bail!("expected {:?} representation, got {:?}", a, b);
+                    }
+                }
+                self.bump_ntests();
+            }
         }
         Ok(())
     }
@@ -158,7 +201,10 @@ impl TestState {
 
 mod kw {
     wast::custom_keyword!(assert_invalid);
+    wast::custom_keyword!(assert_representable);
     wast::custom_keyword!(witx);
+    wast::custom_keyword!(eq);
+    wast::custom_keyword!(noteq);
 }
 
 struct Witxt<'a> {
@@ -182,6 +228,12 @@ enum WitxtDirective<'a> {
         witx: Witx<'a>,
         message: &'a str,
     },
+    AssertRepEquality {
+        span: wast::Span,
+        repr: RepEquality,
+        t1: (wast::Id<'a>, &'a str),
+        t2: (wast::Id<'a>, &'a str),
+    },
 }
 
 impl WitxtDirective<'_> {
@@ -189,6 +241,7 @@ impl WitxtDirective<'_> {
         match self {
             WitxtDirective::Witx(w) => w.span,
             WitxtDirective::AssertInvalid { span, .. } => *span,
+            WitxtDirective::AssertRepEquality { span, .. } => *span,
         }
     }
 }
@@ -205,6 +258,14 @@ impl<'a> Parse<'a> for WitxtDirective<'a> {
                 witx: parser.parens(|p| p.parse())?,
                 message: parser.parse()?,
             })
+        } else if l.peek::<kw::assert_representable>() {
+            let span = parser.parse::<kw::assert_representable>()?.0;
+            Ok(WitxtDirective::AssertRepEquality {
+                span,
+                repr: parser.parse()?,
+                t1: (parser.parse()?, parser.parse()?),
+                t2: (parser.parse()?, parser.parse()?),
+            })
         } else {
             Err(l.error())
         }
@@ -213,6 +274,7 @@ impl<'a> Parse<'a> for WitxtDirective<'a> {
 
 struct Witx<'a> {
     span: wast::Span,
+    id: Option<wast::Id<'a>>,
     decls: Vec<witx::parser::Documented<'a, witx::parser::DeclSyntax<'a>>>,
 }
 
@@ -234,10 +296,32 @@ impl Witx<'_> {
 impl<'a> Parse<'a> for Witx<'a> {
     fn parse(parser: Parser<'a>) -> parser::Result<Self> {
         let span = parser.parse::<kw::witx>()?.0;
+        let id = parser.parse()?;
         let mut decls = Vec::new();
         while !parser.is_empty() {
             decls.push(parser.parens(|p| p.parse())?);
         }
-        Ok(Witx { span, decls })
+        Ok(Witx { id, span, decls })
+    }
+}
+
+#[derive(Debug)]
+enum RepEquality {
+    Eq,
+    NotEq,
+}
+
+impl<'a> Parse<'a> for RepEquality {
+    fn parse(parser: Parser<'a>) -> parser::Result<Self> {
+        let mut l = parser.lookahead1();
+        if l.peek::<kw::eq>() {
+            parser.parse::<kw::eq>()?;
+            Ok(RepEquality::Eq)
+        } else if l.peek::<kw::noteq>() {
+            parser.parse::<kw::noteq>()?;
+            Ok(RepEquality::NotEq)
+        } else {
+            Err(l.error())
+        }
     }
 }
diff --git a/tools/witx/tests/witxt/union.witxt b/tools/witx/tests/witxt/union.witxt
new file mode 100644
index 000000000..2ea460dff
--- /dev/null
+++ b/tools/witx/tests/witxt/union.witxt
@@ -0,0 +1,96 @@
+
+(witx
+  (typename $tag (enum u8 $c))
+  (typename $u (union $tag (field $c u8)))
+)
+
+(witx
+  (typename $tag (enum u8 $a $b))
+  (typename $u (union $tag (empty $a) (field $b u16)))
+)
+
+(witx
+  (typename $tag (enum u8 $a $b))
+  (typename $u (union $tag (empty $a) (empty $b)))
+)
+
+
+(witx
+ (typename $tag (enum u32 $a $b $c $d $e $f $g $h $i $j $k $l $m))
+ (typename $u
+  (union $tag
+   (field $a u8)
+   (field $b u16)
+   (field $c u32)
+   (field $d u64)
+   (field $e s8)
+   (field $f s16)
+   (field $g s32)
+   (field $h s64)
+   (field $i f32)
+   (field $j f64)
+   (field $k (@witx usize))
+   (field $l char8)
+   (empty $m)
+  )
+ )
+)
+
+(assert_invalid
+  (witx (typename $u (union $tag (field $a u8) (field $b u16))))
+  "Unknown name `tag`"
+)
+
+(assert_invalid
+  (witx
+    (typename $tag u32)
+    (typename $u (union $tag (field $a u8) (field $b u16)))
+  )
+  "Wrong kind of name `tag`: expected enum, got builtin"
+)
+
+(assert_invalid
+  (witx
+    (typename $tag (enum u8 $c))
+    (typename $u (union $tag (field $b u8)))
+  )
+  "Invalid union field `b`: does not correspond to variant in tag `tag`"
+)
+
+(assert_invalid
+  (witx
+    (typename $tag (enum u8 $c))
+    (typename $u (union $tag (field $c f32) (field $b u8)))
+  )
+  "Invalid union field `b`: does not correspond to variant in tag `tag`"
+)
+
+(assert_invalid
+  (witx
+    (typename $tag (enum u8 $c $d))
+    (typename $u (union $tag (field $c f32)))
+  )
+  "Invalid union field `d`: missing variants from tag `tag`"
+)
+
+(witx $d1
+  (typename $tag (enum u8 $a $b))
+  (typename $u (union $tag (field $a u8) (field $b u16)))
+)
+
+(witx $d2
+  (typename $tag (enum u8 $a $b))
+  (typename $u (union $tag (field $b u16) (field $a u8)))
+)
+
+;; These two unions should be represented the same:
+(assert_representable eq $d1 "u" $d2 "u")
+(assert_representable eq $d2 "u" $d1 "u")
+
+;; Tag order doesnt matter for validation, but does for rep equality
+(witx $d3
+  (typename $tag (enum u8 $b $a))
+  (typename $u (union $tag (field $b u16) (field $a u8)))
+)
+
+(assert_representable noteq $d3 "u" $d1 "u")

From 47e7024d5e738c453ab188b53cafc88abcd61553 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Wed, 10 Feb 2021 09:29:13 -0800
Subject: [PATCH 3/7] Add documentation/roundtrip testing to all documents

---
 tools/witx/tests/witxt.rs | 56 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/tools/witx/tests/witxt.rs b/tools/witx/tests/witxt.rs
index 4e448bee8..7caf195a5 100644
--- a/tools/witx/tests/witxt.rs
+++ b/tools/witx/tests/witxt.rs
@@ -14,7 +14,7 @@ use std::path::{Path, PathBuf};
 use std::str;
 use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
 use wast::parser::{self, Parse, ParseBuffer, Parser};
-use witx::Representable;
+use witx::{Documentation, Representable};
 
 fn main() {
     let tests = find_tests();
@@ -146,10 +146,12 @@ impl WitxtRunner<'_> {
         test: &Path,
         directive: WitxtDirective,
     ) -> Result<()> {
+        self.bump_ntests();
         match directive {
             WitxtDirective::Witx(witx) => {
                 let doc = witx.document(contents, test)?;
-                self.bump_ntests();
+                self.assert_roundtrip(&doc)?;
+                self.assert_md(&doc)?;
                 if let Some(name) = witx.id {
                     self.documents.insert(name.name().to_string(), doc);
                 }
@@ -162,7 +164,6 @@ impl WitxtRunner<'_> {
                 if !err.contains(message) {
                     bail!("expected error {:?}\nfound error {}", message, err);
                 }
-                self.bump_ntests();
             }
             WitxtDirective::AssertRepEquality { repr, t1, t2, .. } => {
                 let (t1m, t1t) = t1;
@@ -188,12 +189,59 @@ impl WitxtRunner<'_> {
                         bail!("expected {:?} representation, got {:?}", a, b);
                     }
                 }
-                self.bump_ntests();
             }
         }
         Ok(())
     }
 
+    fn assert_roundtrip(&self, doc: &witx::Document) -> Result<()> {
+        self.bump_ntests();
+        let back_to_sexprs = format!("{}", doc);
+        let doc2 = witx::parse(&back_to_sexprs)?;
+        if *doc == doc2 {
+            return Ok(());
+        }
+
+        // Try to get a more specific error message that isn't thousands of
+        // lines long of debug representations.
+        for type_ in doc.typenames() {
+            let type2 = match doc2.typename(&type_.name) {
+                Some(t) => t,
+                None => bail!("doc2 missing datatype"),
+            };
+            if type_ != type2 {
+                bail!("{:?} != {:?}", type_, type2);
+            }
+        }
+        for mod_ in doc.modules() {
+            let mod2 = match doc2.module(&mod_.name) {
+                Some(m) => m,
+                None => bail!("doc2 missing module"),
+            };
+            for import in mod_.imports() {
+                let import2 = match mod2.import(&import.name) {
+                    Some(i) => i,
+                    None => bail!("mod2 missing import"),
+                };
+                assert_eq!(import, import2);
+            }
+            for func in mod_.funcs() {
+                let func2 = match mod2.func(&func.name) {
+                    Some(f) => f,
+                    None => bail!("mod2 missing func"),
+                };
+                assert_eq!(func, func2);
+            }
+        }
+        bail!("{:?} != {:?}", doc, doc2)
+    }
+
+    fn assert_md(&self, doc: &witx::Document) -> Result<()> {
+        self.bump_ntests();
+        doc.to_md();
+        Ok(())
+    }
+
     fn bump_ntests(&self) {
         self.ntests.fetch_add(1, SeqCst);
     }

From 3d853674d20146b40983e509a02997ce529e8a42 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Wed, 10 Feb 2021 09:46:50 -0800
Subject: [PATCH 4/7] Translate multi-module test suite to `*.witxt`

---
 tools/witx/tests/multimodule.rs               | 58 ----------------
 tools/witx/tests/witxt.rs                     | 68 +++++++++++++------
 tools/witx/tests/witxt/multimodule.witxt      | 22 ++++++
 .../{ => witxt}/multimodule/redefine_a.witx   |  0
 .../tests/{ => witxt}/multimodule/type_a.witx |  0
 .../tests/{ => witxt}/multimodule/type_b.witx |  0
 .../tests/{ => witxt}/multimodule/type_c.witx |  0
 7 files changed, 70 insertions(+), 78 deletions(-)
 delete mode 100644 tools/witx/tests/multimodule.rs
 create mode 100644 tools/witx/tests/witxt/multimodule.witxt
 rename tools/witx/tests/{ => witxt}/multimodule/redefine_a.witx (100%)
 rename tools/witx/tests/{ => witxt}/multimodule/type_a.witx (100%)
 rename tools/witx/tests/{ => witxt}/multimodule/type_b.witx (100%)
 rename tools/witx/tests/{ => witxt}/multimodule/type_c.witx (100%)

diff --git a/tools/witx/tests/multimodule.rs b/tools/witx/tests/multimodule.rs
deleted file mode 100644
index a6c773363..000000000
--- a/tools/witx/tests/multimodule.rs
+++ /dev/null
@@ -1,58 +0,0 @@
-use witx::{load, BuiltinType, Id, Type, TypeRef};
-
-#[test]
-fn validate_multimodule() {
-    // B uses A, and C uses A.
-    let doc = load(&[
-        "tests/multimodule/type_b.witx",
-        "tests/multimodule/type_c.witx",
-    ])
-    .unwrap_or_else(|e| panic!("failed to validate: {}", e));
-
-    //println!("{}", doc);
-
-    // Check that the `a` both modules use is what we expect:
-    let type_a = doc.typename(&Id::new("a")).expect("type a exists");
-    assert_eq!(
-        *type_a.type_(),
-        Type::Builtin(BuiltinType::U32 {
-            lang_ptr_size: false
-        })
-    );
-
-    // `b` is a struct with a single member of type `a`
-    let type_b = doc.typename(&Id::new("b")).expect("type b exists");
-    match &*type_b.type_() {
-        Type::Record(record) => {
-            assert_eq!(record.members.len(), 1);
-            assert_eq!(
-                record.members.get(0).unwrap().tref,
-                TypeRef::Name(type_a.clone())
-            );
-        }
-        _ => panic!("b is a struct"),
-    }
-
-    // `c` is a struct with a two members of type `a`
-    let type_c = doc.typename(&Id::new("c")).expect("type c exists");
-    match &*type_c.type_() {
-        Type::Record(record) => {
-            assert_eq!(record.members.len(), 2);
-            assert_eq!(
-                record.members.get(0).unwrap().tref,
-                TypeRef::Name(type_a.clone())
-            );
-            assert_eq!(record.members.get(1).unwrap().tref, TypeRef::Name(type_a));
-        }
-        _ => panic!("c is a struct"),
-    }
-}
-
-#[test]
-fn multimodule_reject_redefinition() {
-    assert!(load(&[
-        "tests/multimodule/type_a.witx",
-        "tests/multimodule/redefine_a.witx",
-    ])
-    .is_err())
-}
diff --git a/tools/witx/tests/witxt.rs b/tools/witx/tests/witxt.rs
index 7caf195a5..b095730ce 100644
--- a/tools/witx/tests/witxt.rs
+++ b/tools/witx/tests/witxt.rs
@@ -165,7 +165,7 @@ impl WitxtRunner<'_> {
                     bail!("expected error {:?}\nfound error {}", message, err);
                 }
             }
-            WitxtDirective::AssertRepEquality { repr, t1, t2, .. } => {
+            WitxtDirective::AssertRepresentable { repr, t1, t2, .. } => {
                 let (t1m, t1t) = t1;
                 let (t2m, t2t) = t2;
                 let t1d = self
@@ -253,6 +253,7 @@ mod kw {
     wast::custom_keyword!(witx);
     wast::custom_keyword!(eq);
     wast::custom_keyword!(noteq);
+    wast::custom_keyword!(load);
 }
 
 struct Witxt<'a> {
@@ -276,7 +277,7 @@ enum WitxtDirective<'a> {
         witx: Witx<'a>,
         message: &'a str,
     },
-    AssertRepEquality {
+    AssertRepresentable {
         span: wast::Span,
         repr: RepEquality,
         t1: (wast::Id<'a>, &'a str),
@@ -288,8 +289,8 @@ impl WitxtDirective<'_> {
     fn span(&self) -> wast::Span {
         match self {
             WitxtDirective::Witx(w) => w.span,
-            WitxtDirective::AssertInvalid { span, .. } => *span,
-            WitxtDirective::AssertRepEquality { span, .. } => *span,
+            WitxtDirective::AssertInvalid { span, .. }
+            | WitxtDirective::AssertRepresentable { span, .. } => *span,
         }
     }
 }
@@ -308,7 +309,7 @@ impl<'a> Parse<'a> for WitxtDirective<'a> {
             })
         } else if l.peek::<kw::assert_representable>() {
             let span = parser.parse::<kw::assert_representable>()?.0;
-            Ok(WitxtDirective::AssertRepEquality {
+            Ok(WitxtDirective::AssertRepresentable {
                 span,
                 repr: parser.parse()?,
                 t1: (parser.parse()?, parser.parse()?),
@@ -323,21 +324,35 @@ impl<'a> Parse<'a> for WitxtDirective<'a> {
 struct Witx<'a> {
     span: wast::Span,
     id: Option<wast::Id<'a>>,
-    decls: Vec<witx::parser::Documented<'a, witx::parser::DeclSyntax<'a>>>,
+    def: WitxDef<'a>,
+}
+
+enum WitxDef<'a> {
+    Fs(Vec<&'a str>),
+    Inline(Vec<witx::parser::Documented<'a, witx::parser::DeclSyntax<'a>>>),
 }
 
 impl Witx<'_> {
     fn document(&self, contents: &str, file: &Path) -> Result<witx::Document> {
-        let mut validator = witx::DocValidation::new();
-        let mut definitions = Vec::new();
-        for decl in self.decls.iter() {
-            let def = validator
-                .scope(contents, file)
-                .validate_decl(&decl.item, &decl.comments)
-                .map_err(witx::WitxError::Validation)?;
-            definitions.push(def);
+        match &self.def {
+            WitxDef::Inline(decls) => {
+                let mut validator = witx::DocValidation::new();
+                let mut definitions = Vec::new();
+                for decl in decls {
+                    let def = validator
+                        .scope(contents, file)
+                        .validate_decl(&decl.item, &decl.comments)
+                        .map_err(witx::WitxError::Validation)?;
+                    definitions.push(def);
+                }
+                Ok(validator.into_document(definitions))
+            }
+            WitxDef::Fs(paths) => {
+                let parent = file.parent().unwrap();
+                let paths = paths.iter().map(|p| parent.join(p)).collect::<Vec<_>>();
+                Ok(witx::load(&paths)?)
+            }
         }
-        Ok(validator.into_document(definitions))
     }
 }
 
@@ -345,11 +360,24 @@ impl<'a> Parse<'a> for Witx<'a> {
     fn parse(parser: Parser<'a>) -> parser::Result<Self> {
         let span = parser.parse::<kw::witx>()?.0;
         let id = parser.parse()?;
-        let mut decls = Vec::new();
-        while !parser.is_empty() {
-            decls.push(parser.parens(|p| p.parse())?);
-        }
-        Ok(Witx { id, span, decls })
+
+        let def = if parser.peek2::<kw::load>() {
+            parser.parens(|p| {
+                p.parse::<kw::load>()?;
+                let mut paths = Vec::new();
+                while !p.is_empty() {
+                    paths.push(p.parse()?);
+                }
+                Ok(WitxDef::Fs(paths))
+            })?
+        } else {
+            let mut decls = Vec::new();
+            while !parser.is_empty() {
+                decls.push(parser.parens(|p| p.parse())?);
+            }
+            WitxDef::Inline(decls)
+        };
+        Ok(Witx { id, span, def })
     }
 }
 
diff --git a/tools/witx/tests/witxt/multimodule.witxt b/tools/witx/tests/witxt/multimodule.witxt
new file mode 100644
index 000000000..ba1e96c9f
--- /dev/null
+++ b/tools/witx/tests/witxt/multimodule.witxt
@@ -0,0 +1,22 @@
+;; B uses A, and C uses A.
+(witx $multi
+  (load "multimodule/type_b.witx" "multimodule/type_c.witx")
+)
+
+(witx $reference
+  (typename $a u32)
+  (typename $b (struct (field $member_a $a)))
+  (typename $c (struct (field $first_a $a) (field $second_a $a)))
+)
+
+(assert_representable eq $reference "a" $multi "a")
+(assert_representable eq $reference "b" $multi "b")
+(assert_representable eq $reference "c" $multi "c")
+
+(assert_invalid
+  (witx
+    (load
+          "multimodule/type_a.witx"
+          "multimodule/redefine_a.witx")
+  )
+  "Redefinition of name `a`")
diff --git a/tools/witx/tests/multimodule/redefine_a.witx b/tools/witx/tests/witxt/multimodule/redefine_a.witx
similarity index 100%
rename from tools/witx/tests/multimodule/redefine_a.witx
rename to tools/witx/tests/witxt/multimodule/redefine_a.witx
diff --git a/tools/witx/tests/multimodule/type_a.witx b/tools/witx/tests/witxt/multimodule/type_a.witx
similarity index 100%
rename from tools/witx/tests/multimodule/type_a.witx
rename to tools/witx/tests/witxt/multimodule/type_a.witx
diff --git a/tools/witx/tests/multimodule/type_b.witx b/tools/witx/tests/witxt/multimodule/type_b.witx
similarity index 100%
rename from tools/witx/tests/multimodule/type_b.witx
rename to tools/witx/tests/witxt/multimodule/type_b.witx
diff --git a/tools/witx/tests/multimodule/type_c.witx b/tools/witx/tests/witxt/multimodule/type_c.witx
similarity index 100%
rename from tools/witx/tests/multimodule/type_c.witx
rename to tools/witx/tests/witxt/multimodule/type_c.witx

From 4e55d6b8791fa6f4bd7c3be94811c1ee7a440928 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Wed, 10 Feb 2021 09:52:14 -0800
Subject: [PATCH 5/7] Move wasi tests into `witxt` test suites

---
 tools/witx/tests/{wasi.rs => wasi-docs.rs} | 64 ----------------------
 tools/witx/tests/witxt/wasi.witxt          | 26 +++++++++
 2 files changed, 26 insertions(+), 64 deletions(-)
 rename tools/witx/tests/{wasi.rs => wasi-docs.rs} (58%)
 create mode 100644 tools/witx/tests/witxt/wasi.witxt

diff --git a/tools/witx/tests/wasi.rs b/tools/witx/tests/wasi-docs.rs
similarity index 58%
rename from tools/witx/tests/wasi.rs
rename to tools/witx/tests/wasi-docs.rs
index 27300a26e..6702edfbd 100644
--- a/tools/witx/tests/wasi.rs
+++ b/tools/witx/tests/wasi-docs.rs
@@ -2,24 +2,6 @@ use std::fs;
 use std::path::Path;
 use witx::{self, Documentation};
 
-#[test]
-fn validate_wasi_snapshot() {
-    witx::load(&witx::phases::snapshot().unwrap())
-        .unwrap_or_else(|e| panic!("failed to parse: {}", e));
-}
-
-#[test]
-fn validate_wasi_ephemeral() {
-    witx::load(&witx::phases::ephemeral().unwrap())
-        .unwrap_or_else(|e| panic!("failed to parse: {}", e));
-}
-
-#[test]
-fn validate_wasi_old_snapshot_0() {
-    witx::load(&witx::phases::old::snapshot_0().unwrap())
-        .unwrap_or_else(|e| panic!("failed to parse: {}", e));
-}
-
 #[test]
 fn validate_docs() {
     for phase in &[
@@ -32,52 +14,6 @@ fn validate_docs() {
     }
 }
 
-#[test]
-fn render_roundtrip() {
-    let doc = witx::load(&witx::phases::snapshot().unwrap())
-        .unwrap_or_else(|e| panic!("failed to parse: {}", e));
-
-    let back_to_sexprs = format!("{}", doc);
-    println!("{}", back_to_sexprs);
-
-    let doc2 = witx::parse(&back_to_sexprs)
-        .map_err(|e| e.report_with(&witx::MockFs::new(&[("-", &back_to_sexprs)])))
-        .unwrap();
-
-    // I'd just assert_eq, but when it fails the debug print is thousands of lines long and impossible
-    // to figure out where they are unequal.
-    if doc != doc2 {
-        for type_ in doc.typenames() {
-            let type2 = doc2.typename(&type_.name).expect("doc2 missing datatype");
-            assert_eq!(type_, type2);
-        }
-        for mod_ in doc.modules() {
-            let mod2 = doc2.module(&mod_.name).expect("doc2 missing module");
-            for import in mod_.imports() {
-                let import2 = mod2.import(&import.name).expect("mod2 missing import");
-                assert_eq!(import, import2);
-            }
-            for func in mod_.funcs() {
-                let func2 = mod2.func(&func.name).expect("mod2 missing func");
-                assert_eq!(func, func2);
-            }
-        }
-    }
-    // This should be equivalent to the above, but just in case some code changes where it isnt:
-    assert_eq!(doc, doc2);
-}
-
-#[test]
-fn document_wasi_snapshot() {
-    use witx::Documentation;
-    println!(
-        "{}",
-        witx::load(&witx::phases::snapshot().unwrap())
-            .unwrap_or_else(|e| panic!("failed to parse: {}", e))
-            .to_md()
-    );
-}
-
 fn dos2unix(s: &str) -> String {
     let mut t = String::new();
     t.reserve(s.len());
diff --git a/tools/witx/tests/witxt/wasi.witxt b/tools/witx/tests/witxt/wasi.witxt
new file mode 100644
index 000000000..f654a7ee3
--- /dev/null
+++ b/tools/witx/tests/witxt/wasi.witxt
@@ -0,0 +1,26 @@
+;; Ensure that all current witx definitions in this repository load, parse,
+;; roundtrip, and are documentable.
+
+(witx
+  (load "../../../../phases/old/snapshot_0/witx/wasi_unstable.witx"))
+(witx
+  (load "../../../../phases/snapshot/witx/wasi_snapshot_preview1.witx"))
+
+(witx
+  (load
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_args.witx"
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_clock.witx"
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_environ.witx"
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_fd.witx"
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_path.witx"
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_poll.witx"
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_proc.witx"
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_random.witx"
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_sched.witx"
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_sock.witx"
+  )
+)
+;; should be singularly-loadable as well
+(witx
+  (load
+    "../../../../phases/ephemeral/witx/wasi_ephemeral_fd.witx"))

From b8c1056686aafa4d80fd05a7f29f2075c01fdb0c Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Wed, 10 Feb 2021 10:05:59 -0800
Subject: [PATCH 6/7] Convert representation tests to `*.witxt`

---
 tools/witx/src/representation.rs            | 73 ---------------------
 tools/witx/tests/witxt.rs                   | 10 ++-
 tools/witx/tests/witxt/representation.witxt | 58 ++++++++++++++++
 3 files changed, 66 insertions(+), 75 deletions(-)
 create mode 100644 tools/witx/tests/witxt/representation.witxt

diff --git a/tools/witx/src/representation.rs b/tools/witx/src/representation.rs
index 99f73f5b6..7315192e5 100644
--- a/tools/witx/src/representation.rs
+++ b/tools/witx/src/representation.rs
@@ -157,76 +157,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
-        );
-    }
-}
diff --git a/tools/witx/tests/witxt.rs b/tools/witx/tests/witxt.rs
index b095730ce..5dabf1b92 100644
--- a/tools/witx/tests/witxt.rs
+++ b/tools/witx/tests/witxt.rs
@@ -178,12 +178,13 @@ impl WitxtRunner<'_> {
                     .ok_or_else(|| anyhow!("no document named {:?}", t2m.name()))?;
                 let t1 = t1d
                     .typename(&witx::Id::new(t1t))
-                    .ok_or_else(|| anyhow!("no document named {:?}", t1t))?;
+                    .ok_or_else(|| anyhow!("no type named {:?}", t1t))?;
                 let t2 = t2d
                     .typename(&witx::Id::new(t2t))
-                    .ok_or_else(|| anyhow!("no document named {:?}", t2t))?;
+                    .ok_or_else(|| anyhow!("no type named {:?}", t2t))?;
                 match (repr, t1.type_().representable(&t2.type_())) {
                     (RepEquality::Eq, witx::RepEquality::Eq)
+                    | (RepEquality::Superset, witx::RepEquality::Superset)
                     | (RepEquality::NotEq, witx::RepEquality::NotEq) => {}
                     (a, b) => {
                         bail!("expected {:?} representation, got {:?}", a, b);
@@ -254,6 +255,7 @@ mod kw {
     wast::custom_keyword!(eq);
     wast::custom_keyword!(noteq);
     wast::custom_keyword!(load);
+    wast::custom_keyword!(superset);
 }
 
 struct Witxt<'a> {
@@ -385,6 +387,7 @@ impl<'a> Parse<'a> for Witx<'a> {
 enum RepEquality {
     Eq,
     NotEq,
+    Superset,
 }
 
 impl<'a> Parse<'a> for RepEquality {
@@ -396,6 +399,9 @@ impl<'a> Parse<'a> for RepEquality {
         } else if l.peek::<kw::noteq>() {
             parser.parse::<kw::noteq>()?;
             Ok(RepEquality::NotEq)
+        } else if l.peek::<kw::superset>() {
+            parser.parse::<kw::superset>()?;
+            Ok(RepEquality::Superset)
         } else {
             Err(l.error())
         }
diff --git a/tools/witx/tests/witxt/representation.witxt b/tools/witx/tests/witxt/representation.witxt
new file mode 100644
index 000000000..51f5fe373
--- /dev/null
+++ b/tools/witx/tests/witxt/representation.witxt
@@ -0,0 +1,58 @@
+;; type names don't matter
+(witx $a
+  (typename $a (flags u32 $b $c)))
+(witx $b
+  (typename $b (flags u32 $b $c)))
+
+(assert_representable eq $a "a" $b "b")
+(assert_representable eq $b "b" $a "a")
+
+;; flags
+(witx $a
+  (typename $a (flags u32 $b $c)))
+(witx $b
+  (typename $b (flags u32 $b $c $d)))
+
+(assert_representable noteq $b "b" $a "a")
+(assert_representable superset $a "a" $b "b")
+
+(witx $c
+  (typename $c (flags u32 $b $e)))
+(assert_representable noteq $a "a" $c "c")
+(assert_representable noteq $c "c" $a "a")
+
+(witx $d
+  (typename $d (flags u16 $b $c)))
+(assert_representable noteq $a "a" $d "d")
+(assert_representable superset $d "d" $a "a")
+(assert_representable superset $d "d" $b "b")
+
+;; enums
+(witx $a
+  (typename $a (enum u32 $b $c)))
+(witx $b
+  (typename $b (enum u32 $b $c $d)))
+(assert_representable superset $a "a" $b "b")
+(assert_representable noteq $b "b" $a "a")
+
+(witx $c
+  (typename $c (enum u16 $b $c)))
+(assert_representable superset $c "c" $a "a")
+(assert_representable superset $c "c" $b "b")
+
+;; unions
+(witx $a
+  (typename $tag (enum u8 $b $c))
+  (typename $a (union $tag (field $b u32) (field $c f32))))
+(witx $b
+  (typename $tag (enum u8 $b $c $d))
+  (typename $b (union $tag (field $b u32) (field $c f32) (field $d f64))))
+(assert_representable superset $a "a" $b "b")
+(assert_representable noteq $b "b" $a "a")
+
+(witx $c
+  (typename $tag (enum u8 $b $c))
+  (typename $c (union $tag (field $c f32) (field $b u32))))
+(assert_representable eq $a "a" $c "c")
+(assert_representable eq $c "c" $a "a")
+(assert_representable superset $c "c" $b "b")

From f22b62517df658ef9b737db5c81257b9b60aec45 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Thu, 11 Feb 2021 07:55:14 -0800
Subject: [PATCH 7/7] Rebased onto origin/main

---
 phases/ephemeral/docs.md                    |  4 +-
 tools/witx/src/parser.rs                    |  5 +-
 tools/witx/src/representation.rs            |  8 ++-
 tools/witx/src/validate.rs                  | 28 ++++++--
 tools/witx/tests/witxt.rs                   | 10 +--
 tools/witx/tests/witxt/anonymous.witxt      | 16 ++---
 tools/witx/tests/witxt/multimodule.witxt    |  4 +-
 tools/witx/tests/witxt/representation.witxt | 32 +++++----
 tools/witx/tests/witxt/union.witxt          | 79 +++++++++++----------
 9 files changed, 103 insertions(+), 83 deletions(-)

diff --git a/phases/ephemeral/docs.md b/phases/ephemeral/docs.md
index 2cd2d1361..a3c0d0a47 100644
--- a/phases/ephemeral/docs.md
+++ b/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.
 
diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs
index 325eb1d35..19d1a0303 100644
--- a/tools/witx/src/parser.rs
+++ b/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 })
     }
diff --git a/tools/witx/src/representation.rs b/tools/witx/src/representation.rs
index 7315192e5..589b64e39 100644
--- a/tools/witx/src/representation.rs
+++ b/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) {
diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs
index 7460d8e76..ade02282b 100644
--- a/tools/witx/src/validate.rs
+++ b/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;
@@ -443,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(),
@@ -471,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 })
     }
 
diff --git a/tools/witx/tests/witxt.rs b/tools/witx/tests/witxt.rs
index 5dabf1b92..1056947a3 100644
--- a/tools/witx/tests/witxt.rs
+++ b/tools/witx/tests/witxt.rs
@@ -150,7 +150,8 @@ impl WitxtRunner<'_> {
         match directive {
             WitxtDirective::Witx(witx) => {
                 let doc = witx.document(contents, test)?;
-                self.assert_roundtrip(&doc)?;
+                self.assert_roundtrip(&doc)
+                    .context("failed to round-trip the document")?;
                 self.assert_md(&doc)?;
                 if let Some(name) = witx.id {
                     self.documents.insert(name.name().to_string(), doc);
@@ -211,7 +212,7 @@ impl WitxtRunner<'_> {
                 None => bail!("doc2 missing datatype"),
             };
             if type_ != type2 {
-                bail!("{:?} != {:?}", type_, type2);
+                bail!("types are not equal\n{:?}\n   !=\n{:?}", type_, type2);
             }
         }
         for mod_ in doc.modules() {
@@ -341,11 +342,10 @@ impl Witx<'_> {
                 let mut validator = witx::DocValidation::new();
                 let mut definitions = Vec::new();
                 for decl in decls {
-                    let def = validator
+                    validator
                         .scope(contents, file)
-                        .validate_decl(&decl.item, &decl.comments)
+                        .validate_decl(&decl.item, &decl.comments, &mut definitions)
                         .map_err(witx::WitxError::Validation)?;
-                    definitions.push(def);
                 }
                 Ok(validator.into_document(definitions))
             }
diff --git a/tools/witx/tests/witxt/anonymous.witxt b/tools/witx/tests/witxt/anonymous.witxt
index d7a36c12f..0ed3bc0d2 100644
--- a/tools/witx/tests/witxt/anonymous.witxt
+++ b/tools/witx/tests/witxt/anonymous.witxt
@@ -3,26 +3,25 @@
 
 (assert_invalid
   (witx
-    (typename $a (@witx pointer (struct (field $b u8))))
+    (typename $a (@witx pointer (record (field $b u8))))
   )
   "Anonymous structured types")
 
 (assert_invalid
   (witx
-    (typename $tag (enum u8 $b))
-    (typename $a (@witx pointer (union $tag (field $b u8))))
+    (typename $a (@witx pointer (union)))
   )
   "Anonymous structured types")
 
 (assert_invalid
   (witx
-    (typename $a (@witx pointer (enum u32 $b)))
+    (typename $a (@witx pointer (enum $b)))
   )
   "Anonymous structured types")
 
 (assert_invalid
   (witx
-    (typename $a (@witx pointer (flags u32 $b)))
+    (typename $a (@witx pointer (flags $b)))
   )
   "Anonymous structured types")
 
@@ -34,13 +33,14 @@
 
 (assert_invalid
   (witx
-    (typename $a (struct (field $b (struct (field $c u8)))))
+    (typename $a (record (field $b (record (field $c u8)))))
   )
   "Anonymous structured types")
 
 (assert_invalid
   (witx
-    (typename $tag (enum u8 $c)) (typename $a (struct (field $b (union $tag (field $c u8)))))
+    (typename $tag (enum $c))
+    (typename $a (record (field $b (union))))
   )
   "Anonymous structured types")
 
@@ -53,4 +53,4 @@
   (typename $a (@witx pointer (@witx const_pointer u8))))
 
 (witx
-  (typename $a (struct (field $b (@witx pointer u8)))))
+  (typename $a (record (field $b (@witx pointer u8)))))
diff --git a/tools/witx/tests/witxt/multimodule.witxt b/tools/witx/tests/witxt/multimodule.witxt
index ba1e96c9f..d8b6559ac 100644
--- a/tools/witx/tests/witxt/multimodule.witxt
+++ b/tools/witx/tests/witxt/multimodule.witxt
@@ -5,8 +5,8 @@
 
 (witx $reference
   (typename $a u32)
-  (typename $b (struct (field $member_a $a)))
-  (typename $c (struct (field $first_a $a) (field $second_a $a)))
+  (typename $b (record (field $member_a $a)))
+  (typename $c (record (field $first_a $a) (field $second_a $a)))
 )
 
 (assert_representable eq $reference "a" $multi "a")
diff --git a/tools/witx/tests/witxt/representation.witxt b/tools/witx/tests/witxt/representation.witxt
index 51f5fe373..37d20a70a 100644
--- a/tools/witx/tests/witxt/representation.witxt
+++ b/tools/witx/tests/witxt/representation.witxt
@@ -1,58 +1,60 @@
 ;; type names don't matter
 (witx $a
-  (typename $a (flags u32 $b $c)))
+  (typename $a (flags (@witx bitflags u8) $b $c)))
 (witx $b
-  (typename $b (flags u32 $b $c)))
+  (typename $b (flags (@witx bitflags u8) $b $c)))
 
 (assert_representable eq $a "a" $b "b")
 (assert_representable eq $b "b" $a "a")
 
+(; TODO: perhaps add assertions eventually for document-level representability?
 ;; flags
 (witx $a
-  (typename $a (flags u32 $b $c)))
+  (typename $a (flags (@witx bitflags u8) $b $c)))
 (witx $b
-  (typename $b (flags u32 $b $c $d)))
+  (typename $b (flags (@witx bitflags u8) $b $c $d)))
 
 (assert_representable noteq $b "b" $a "a")
 (assert_representable superset $a "a" $b "b")
 
 (witx $c
-  (typename $c (flags u32 $b $e)))
+  (typename $c (flags (@witx bitflags u8) $b $e)))
 (assert_representable noteq $a "a" $c "c")
 (assert_representable noteq $c "c" $a "a")
 
 (witx $d
-  (typename $d (flags u16 $b $c)))
+  (typename $d (flags (@witx bitflags u16) $b $c)))
 (assert_representable noteq $a "a" $d "d")
 (assert_representable superset $d "d" $a "a")
 (assert_representable superset $d "d" $b "b")
+;)
 
 ;; enums
 (witx $a
-  (typename $a (enum u32 $b $c)))
+  (typename $a (enum $b $c)))
 (witx $b
-  (typename $b (enum u32 $b $c $d)))
+  (typename $b (enum $b $c $d)))
 (assert_representable superset $a "a" $b "b")
 (assert_representable noteq $b "b" $a "a")
 
 (witx $c
-  (typename $c (enum u16 $b $c)))
+  (typename $c (enum (@witx tag u16) $b $c)))
 (assert_representable superset $c "c" $a "a")
 (assert_representable superset $c "c" $b "b")
 
 ;; unions
 (witx $a
-  (typename $tag (enum u8 $b $c))
-  (typename $a (union $tag (field $b u32) (field $c f32))))
+  (typename $tag (enum $b $c))
+  (typename $a (union (@witx tag $tag) u32 f32)))
 (witx $b
-  (typename $tag (enum u8 $b $c $d))
-  (typename $b (union $tag (field $b u32) (field $c f32) (field $d f64))))
+  (typename $tag (enum $b $c $d))
+  (typename $b (union (@witx tag $tag) u32 f32 f64)))
 (assert_representable superset $a "a" $b "b")
 (assert_representable noteq $b "b" $a "a")
 
 (witx $c
-  (typename $tag (enum u8 $b $c))
-  (typename $c (union $tag (field $c f32) (field $b u32))))
+  (typename $tag (enum $b $c))
+  (typename $c (variant (@witx tag $tag) (case $c f32) (case $b u32))))
 (assert_representable eq $a "a" $c "c")
 (assert_representable eq $c "c" $a "a")
 (assert_representable superset $c "c" $b "b")
diff --git a/tools/witx/tests/witxt/union.witxt b/tools/witx/tests/witxt/union.witxt
index 2ea460dff..92be3dcd9 100644
--- a/tools/witx/tests/witxt/union.witxt
+++ b/tools/witx/tests/witxt/union.witxt
@@ -1,86 +1,87 @@
 
 (witx
-  (typename $tag (enum u8 $c))
-  (typename $u (union $tag (field $c u8)))
+  (typename $u (union u8))
+)
+(witx
+  (typename $tag (enum (@witx tag u8) $c))
+  (typename $u (union (@witx tag $tag) u8))
 )
 
 (witx
-  (typename $tag (enum u8 $a $b))
-  (typename $u (union $tag (empty $a) (field $b u16)))
+  (typename $tag (enum $a $b))
+  (typename $u (variant (@witx tag $tag) (case $a) (case $b u16)))
 )
 
 (witx
-  (typename $tag (enum u8 $a $b))
-  (typename $u (union $tag (empty $a) (empty $b)))
+  (typename $tag (enum $a $b))
+  (typename $u (variant (@witx tag $tag) (case $a) (case $b)))
 )
 
 
 (witx
- (typename $tag (enum u32 $a $b $c $d $e $f $g $h $i $j $k $l $m))
  (typename $u
-  (union $tag
-   (field $a u8)
-   (field $b u16)
-   (field $c u32)
-   (field $d u64)
-   (field $e s8)
-   (field $f s16)
-   (field $g s32)
-   (field $h s64)
-   (field $i f32)
-   (field $j f64)
-   (field $k (@witx usize))
-   (field $l char8)
-   (empty $m)
+  (union
+   u8
+   u16
+   u32
+   u64
+   s8
+   s16
+   s32
+   s64
+   f32
+   f64
+   (@witx usize)
+   (@witx char8)
   )
  )
 )
 
 (assert_invalid
-  (witx (typename $u (union $tag (field $a u8) (field $b u16))))
+  (witx (typename $u (union (@witx tag $tag) u8 u16)))
   "Unknown name `tag`"
 )
 
 (assert_invalid
   (witx
-    (typename $tag u32)
-    (typename $u (union $tag (field $a u8) (field $b u16)))
+    (typename $tag string)
+    (typename $u (union (@witx tag $tag) u8 u16))
   )
-  "Wrong kind of name `tag`: expected enum, got builtin"
+  "Wrong kind of name `tag`: expected enum or builtin, got list"
 )
 
 (assert_invalid
   (witx
-    (typename $tag (enum u8 $c))
-    (typename $u (union $tag (field $b u8)))
+    (typename $tag (enum $c))
+    (typename $u (variant (@witx tag $tag) (case $b u8)))
   )
   "Invalid union field `b`: does not correspond to variant in tag `tag`"
 )
 
 (assert_invalid
   (witx
-    (typename $tag (enum u8 $c))
-    (typename $u (union $tag (field $c f32) (field $b u8)))
+    (typename $tag (enum $c))
+    (typename $u (union (@witx tag $tag) f32 u8))
   )
-  "Invalid union field `b`: does not correspond to variant in tag `tag`"
+  "Union expected 1 variants, found 2"
 )
 
 (assert_invalid
   (witx
-    (typename $tag (enum u8 $c $d))
-    (typename $u (union $tag (field $c f32)))
+    (typename $tag (enum $c $d))
+    (typename $u (union (@witx tag $tag) f32))
   )
-  "Invalid union field `d`: missing variants from tag `tag`"
+  "Union expected 2 variants, found 1"
 )
 
 (witx $d1
-  (typename $tag (enum u8 $a $b))
-  (typename $u (union $tag (field $a u8) (field $b u16)))
+  (typename $tag (enum $a $b))
+  (typename $u (union (@witx tag $tag) u8 u16))
 )
 
 (witx $d2
-  (typename $tag (enum u8 $a $b))
-  (typename $u (union $tag (field $b u16) (field $a u8)))
+  (typename $tag (enum $a $b))
+  (typename $u (variant (@witx tag $tag) (case $b u16) (case $a u8)))
 )
 
 ;; These two unions should be represented the same:
@@ -89,8 +90,8 @@
 
 ;; Tag order doesnt matter for validation, but does for rep equality
 (witx $d3
-  (typename $tag (enum u8 $b $a))
-  (typename $u (union $tag (field $b u16) (field $a u8)))
+  (typename $tag (enum $b $a))
+  (typename $u (union (@witx tag $tag) u16 u8))
 )
 
 (assert_representable noteq $d3 "u" $d1 "u")