From 75001953b77aff38ea3c4314ed04ab668ff85431 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 4 Feb 2020 16:41:49 -0800 Subject: [PATCH 01/17] unions are now tagged by an enum --- tools/witx/src/ast.rs | 1 + tools/witx/src/parser.rs | 4 +- tools/witx/src/representation.rs | 29 +++++-- tools/witx/src/validate.rs | 85 +++++++++++++++++- tools/witx/tests/anonymous.rs | 8 +- tools/witx/tests/union.rs | 143 +++++++++++++++++++++++++++++++ 6 files changed, 258 insertions(+), 12 deletions(-) create mode 100644 tools/witx/tests/union.rs diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index fcb33ad44..d28f0e7e0 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -247,6 +247,7 @@ pub struct StructMember { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct UnionDatatype { + pub tag: TypeRef, pub variants: Vec, } diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs index 08bc9af5b..dc144b9cf 100644 --- a/tools/witx/src/parser.rs +++ b/tools/witx/src/parser.rs @@ -479,18 +479,20 @@ impl<'a> Parse<'a> for FieldSyntax<'a> { #[derive(Debug, Clone, PartialEq, Eq)] pub struct UnionSyntax<'a> { + pub tag: wast::Id<'a>, pub fields: Vec>>, } impl<'a> Parse<'a> for UnionSyntax<'a> { fn parse(parser: Parser<'a>) -> Result { parser.parse::()?; + let tag = parser.parse()?; let mut fields = Vec::new(); fields.push(parser.parse()?); while !parser.is_empty() { fields.push(parser.parse()?); } - Ok(UnionSyntax { fields }) + Ok(UnionSyntax { tag, fields }) } } diff --git a/tools/witx/src/representation.rs b/tools/witx/src/representation.rs index 3bdd6a792..8ba32a82b 100644 --- a/tools/witx/src/representation.rs +++ b/tools/witx/src/representation.rs @@ -185,9 +185,19 @@ impl Representable for UnionDatatype { } } if by.variants.len() > self.variants.len() { - RepEquality::Superset + // By is a superset of self only if the tags are as well: + if self.tag.type_().representable(&*by.tag.type_()) == RepEquality::Superset { + RepEquality::Superset + } else { + RepEquality::NotEq + } } else { - RepEquality::Eq + // By and self have matching variants, so they are equal if tags are: + if self.tag.type_().representable(&*by.tag.type_()) == RepEquality::Eq { + RepEquality::Eq + } else { + RepEquality::NotEq + } } } } @@ -287,16 +297,25 @@ mod test { #[test] fn union() { - let base = def_type("a", "(typename $a (union (field $b u32) (field $c f32)))"); + let base = def_type( + "a", + "(typename $tag (enum u8 $b $c)) + (typename $a (union $tag (field $b u32) (field $c f32)))", + ); let extra_variant = def_type( "a", - "(typename $a (union (field $b u32) (field $c f32) (field $d f64)))", + "(typename $tag (enum u8 $b $c $d)) + (typename $a (union $tag (field $b u32) (field $c f32) (field $d f64)))", ); assert_eq!(base.representable(&extra_variant), RepEquality::Superset); assert_eq!(extra_variant.representable(&base), RepEquality::NotEq); - let other_ordering = def_type("a", "(typename $a (union (field $c f32) (field $b u32)))"); + let other_ordering = def_type( + "a", + "(typename $tag (enum u8 $b $c)) + (typename $a (union $tag (field $c f32) (field $b u32)))", + ); assert_eq!(base.representable(&other_ordering), RepEquality::Eq); assert_eq!(other_ordering.representable(&base), RepEquality::Eq); assert_eq!( diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs index 4bb92aa89..f136499fa 100644 --- a/tools/witx/src/validate.rs +++ b/tools/witx/src/validate.rs @@ -10,7 +10,7 @@ use crate::{ ModuleImportVariant, NamedType, StructDatatype, StructMember, Type, TypePassedBy, TypeRef, UnionDatatype, UnionVariant, }; -use std::collections::HashMap; +use std::collections::{hash_map, HashMap}; use std::path::Path; use std::rc::Rc; use thiserror::Error; @@ -43,6 +43,12 @@ pub enum ValidationError { InvalidFirstResultType { location: Location }, #[error("Anonymous structured types (struct, union, enum, flags, handle) are not permitted")] AnonymousStructure { location: Location }, + #[error("Invalid union field `{name}`: {reason}")] + InvalidUnionField { + name: String, + reason: String, + location: Location, + }, } impl ValidationError { @@ -54,7 +60,8 @@ impl ValidationError { | Recursive { location, .. } | InvalidRepr { location, .. } | InvalidFirstResultType { location, .. } - | AnonymousStructure { location, .. } => { + | AnonymousStructure { location, .. } + | InvalidUnionField { location, .. } => { format!("{}\n{}", location.highlight_source_with(witxio), &self) } NameAlreadyExists { @@ -348,9 +355,42 @@ impl DocValidationScope<'_> { fn validate_union( &self, syntax: &UnionSyntax, - _span: wast::Span, + span: wast::Span, ) -> Result { let mut variant_scope = IdentValidation::new(); + let tag_id = self.get(&syntax.tag)?; + let (tag, tagname, mut variant_name_uses) = match self.doc.entries.get(&tag_id) { + Some(Entry::Typename(weak_ref)) => { + let named_dt = weak_ref.upgrade().expect("weak backref to defined type"); + match &*named_dt.type_() { + Type::Enum(e) => { + let uses = e + .variants + .iter() + .map(|v| (v.name.clone(), false)) + .collect::>(); + let name = named_dt.name.clone(); + Ok((TypeRef::Name(named_dt), name, uses)) + } + other => Err(ValidationError::WrongKindName { + name: syntax.tag.name().to_string(), + location: self.location(syntax.tag.span()), + expected: "enum", + got: other.kind(), + }), + } + } + other => Err(ValidationError::WrongKindName { + name: syntax.tag.name().to_string(), + location: self.location(syntax.tag.span()), + expected: "enum", + got: match other { + Some(e) => e.kind(), + None => "unknown", + }, + }), + }?; + let variants = syntax .fields .iter() @@ -359,11 +399,48 @@ impl DocValidationScope<'_> { .introduce(f.item.name.name(), self.location(f.item.name.span()))?; let tref = self.validate_datatype(&f.item.type_, false, f.item.name.span())?; let docs = f.comments.docs(); + match variant_name_uses.entry(name.clone()) { + hash_map::Entry::Occupied(mut e) => { + if *e.get() { + Err(ValidationError::InvalidUnionField { + name: f.item.name.name().to_string(), + reason: "variant already defined".to_owned(), + location: self.location(f.item.name.span()), + })?; + } else { + e.insert(true); + } + } + hash_map::Entry::Vacant { .. } => Err(ValidationError::InvalidUnionField { + name: f.item.name.name().to_string(), + reason: format!( + "does not correspond to variant in tag `{}`", + tagname.as_str() + ), + location: self.location(f.item.name.span()), + })?, + } Ok(UnionVariant { name, tref, docs }) }) .collect::, _>>()?; - Ok(UnionDatatype { variants }) + let unused_variants = variant_name_uses + .iter() + .filter(|(_k, used)| **used == false) + .map(|(k, _)| k.clone()) + .collect::>(); + if !unused_variants.is_empty() { + Err(ValidationError::InvalidUnionField { + name: unused_variants + .iter() + .map(|i| i.as_str()) + .collect::>() + .join(", "), + reason: format!("missing variants from tag `{}`", tagname.as_str()), + location: self.location(span), + })?; + } + Ok(UnionDatatype { tag, variants }) } fn validate_handle( diff --git a/tools/witx/tests/anonymous.rs b/tools/witx/tests/anonymous.rs index e1976a261..6bf11d883 100644 --- a/tools/witx/tests/anonymous.rs +++ b/tools/witx/tests/anonymous.rs @@ -12,7 +12,9 @@ fn anonymous_types() { let pointer_to_struct = witx::parse("(typename $a (@witx pointer (struct (field $b u8))))"); assert!(is_anonymous_struct_err(pointer_to_struct)); - let pointer_to_union = witx::parse("(typename $a (@witx pointer (union (field $b u8))))"); + let pointer_to_union = witx::parse( + "(typename $tag (enum u8 $b)) (typename $a (@witx pointer (union $tag (field $b u8))))", + ); assert!(is_anonymous_struct_err(pointer_to_union)); let pointer_to_enum = witx::parse("(typename $a (@witx pointer (enum u32 $b)))"); @@ -33,7 +35,9 @@ fn anonymous_types() { let struct_in_struct = witx::parse("(typename $a (struct (field $b (struct (field $c u8)))))"); assert!(is_anonymous_struct_err(struct_in_struct)); - let union_in_struct = witx::parse("(typename $a (struct (field $b (union (field $c u8)))))"); + let union_in_struct = witx::parse( + "(typename $tag (enum u8 $c)) (typename $a (struct (field $b (union $tag (field $c u8)))))", + ); assert!(is_anonymous_struct_err(union_in_struct)); let pointer_in_struct = witx::parse("(typename $a (struct (field $b (@witx pointer u8))))"); diff --git a/tools/witx/tests/union.rs b/tools/witx/tests/union.rs new file mode 100644 index 000000000..3415302ac --- /dev/null +++ b/tools/witx/tests/union.rs @@ -0,0 +1,143 @@ +// Every worker needs a union! Time to organize +use witx::{Id, Representable}; + +#[test] +fn one_variant_union() { + let d = witx::parse( + "(typename $tag (enum u8 $c)) + (typename $u (union $tag (field $c u8)))", + ); + assert!(d.is_ok()); +} + +#[test] +fn two_variant_union() { + let d1 = witx::parse( + "(typename $tag (enum u8 $a $b)) + (typename $u (union $tag (field $a u8) (field $b u16)))", + ); + assert!(d1.is_ok(), "d1 is ok"); + + // Fields can come in whatever order: + let d2 = witx::parse( + "(typename $tag (enum u8 $a $b)) + (typename $u (union $tag (field $b u16) (field $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, but does for rep equality + let d3 = witx::parse( + "(typename $tag (enum u8 $b $a)) + (typename $u (union $tag (field $b u16) (field $a u8)))", + ); + assert!(d3.is_ok(), "d2 is ok"); + let u3 = d3.unwrap().typename(&Id::new("u")).unwrap().type_(); + assert_eq!( + u3.representable(&u1), + witx::RepEquality::NotEq, + "u3 cannot represent u1" + ); +} + +#[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 u32) + (typename $u (union $tag (field $a u8) (field $b u16)))", + ); + let (expected, got) = wrong_kind_name_err(d).expect("wrong kind of tag"); + assert_eq!(expected, "enum"); + assert_eq!(got, "builtin"); +} + +#[test] +fn bad_field_unions() { + let d = witx::parse( + "(typename $tag (enum u8 $c)) + (typename $u (union $tag (field $b u8)))", + ); + let (name, reason) = union_field_err(d).expect("bad field union 1"); + assert_eq!(name, "b", "bad field name union 1"); + assert_eq!( + reason, "does not correspond to variant in tag `tag`", + "reason union 1" + ); + + let d = witx::parse( + "(typename $tag (enum u8 $c)) + (typename $u (union $tag (field $c f32) (field $b u8)))", + ); + let (name, reason) = union_field_err(d).expect("bad field union 2"); + assert_eq!(name, "b", "bad field name union 2"); + assert_eq!( + reason, "does not correspond to variant in tag `tag`", + "reason union 2" + ); + + let d = witx::parse( + "(typename $tag (enum u8 $c $d)) + (typename $u (union $tag (field $c f32)))", + ); + let (name, reason) = union_field_err(d).expect("bad field union 3"); + assert_eq!(name, "d", "bad field name union 3"); + assert_eq!(reason, "missing variants from tag `tag`", "reason union 3"); +} + +fn wrong_kind_name_err( + r: Result, +) -> 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 union_field_err(r: Result) -> Option<(String, String)> { + match r { + Err(witx::WitxError::Validation(witx::ValidationError::InvalidUnionField { + name, + reason, + .. + })) => Some((name, reason)), + Err(e) => { + eprintln!("expected InvalidUnionField ValidationError, got: {:?}", e); + None + } + Ok(_) => { + eprintln!("expected InvalidUnionField ValidationError, got: Ok(witx::Document)"); + None + } + } +} From 975bf431b9949de5bc0e9374a9cbb68f0af0179c Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 4 Feb 2020 17:28:27 -0800 Subject: [PATCH 02/17] union fields may be empty. TODO: write tests --- tools/witx/src/ast.rs | 2 +- tools/witx/src/docs/ast.rs | 6 +++++- tools/witx/src/layout.rs | 34 ++++++++++++++++++++------------ tools/witx/src/parser.rs | 29 ++++++++++++++++++++++++++- tools/witx/src/render.rs | 23 +++++++++++++-------- tools/witx/src/representation.rs | 16 ++++++++++++++- tools/witx/src/validate.rs | 26 ++++++++++++++++-------- 7 files changed, 103 insertions(+), 33 deletions(-) diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index d28f0e7e0..31dd6246e 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -254,7 +254,7 @@ pub struct UnionDatatype { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct UnionVariant { pub name: Id, - pub tref: TypeRef, + pub tref: Option, pub docs: String, } diff --git a/tools/witx/src/docs/ast.rs b/tools/witx/src/docs/ast.rs index a5f2ca952..10dc9f7c2 100644 --- a/tools/witx/src/docs/ast.rs +++ b/tools/witx/src/docs/ast.rs @@ -211,7 +211,11 @@ impl ToMarkdown for UnionDatatype { name, &variant.docs, )); - variant.tref.generate(n.clone()); + if let Some(ref tref) = variant.tref { + tref.generate(n.clone()); + } else { + n.content_ref_mut::().r#type = None; + } } node.content_ref_mut::().r#type = Some(MdType::Union); diff --git a/tools/witx/src/layout.rs b/tools/witx/src/layout.rs index cd3bd7da9..c89a9cdf0 100644 --- a/tools/witx/src/layout.rs +++ b/tools/witx/src/layout.rs @@ -133,24 +133,32 @@ mod test { } impl UnionDatatype { + // TODO also need to expose the layout / existence of the indivdual tag/u members. fn layout(&self, cache: &mut HashMap) -> SizeAlign { + let tag = self.tag.layout(cache); let sas = self .variants .iter() - .map(|v| v.tref.layout(cache)) + .filter_map(|v| v.tref.as_ref().map(|t| t.layout(cache))) .collect::>(); - let size = sas - .iter() - .map(|sa| sa.size) - .max() - .expect("nonzero variants"); - let align = sas - .iter() - .map(|sa| sa.align) - .max() - .expect("nonzero variants"); - let size = align_to(size, align); - SizeAlign { size, align } + if sas.is_empty() { + tag + } else { + let u_size = sas + .iter() + .map(|sa| sa.size) + .max() + .expect("nonzero variants"); + let u_align = sas + .iter() + .map(|sa| sa.align) + .max() + .expect("nonzero variants"); + let u_offset = align_to(tag.size, u_align); + let align = std::cmp::max(tag.align, u_align); + let size = align_to(u_offset + u_size, align); + SizeAlign { size, align } + } } } diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs index dc144b9cf..85f8c8d09 100644 --- a/tools/witx/src/parser.rs +++ b/tools/witx/src/parser.rs @@ -24,6 +24,7 @@ mod kw { wast::custom_keyword!(f32); wast::custom_keyword!(f64); wast::custom_keyword!(field); + wast::custom_keyword!(empty); wast::custom_keyword!(flags); wast::custom_keyword!(handle); wast::custom_keyword!(int); @@ -477,10 +478,36 @@ impl<'a> Parse<'a> for FieldSyntax<'a> { } } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum VariantSyntax<'a> { + Field(FieldSyntax<'a>), + Empty(wast::Id<'a>), +} + +impl<'a> Parse<'a> for VariantSyntax<'a> { + fn parse(parser: Parser<'a>) -> Result { + parser.parens(|p| { + let mut l = p.lookahead1(); + if l.peek::() { + parser.parse::()?; + let name = p.parse()?; + let type_ = p.parse()?; + Ok(VariantSyntax::Field(FieldSyntax { name, type_ })) + } else if l.peek::() { + parser.parse::()?; + let name = p.parse()?; + Ok(VariantSyntax::Empty(name)) + } else { + Err(l.error()) + } + }) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct UnionSyntax<'a> { pub tag: wast::Id<'a>, - pub fields: Vec>>, + pub fields: Vec>>, } impl<'a> Parse<'a> for UnionSyntax<'a> { diff --git a/tools/witx/src/render.rs b/tools/witx/src/render.rs index a1a6ff58a..6bb6e290e 100644 --- a/tools/witx/src/render.rs +++ b/tools/witx/src/render.rs @@ -211,14 +211,21 @@ impl UnionDatatype { .variants .iter() .map(|v| { - SExpr::docs( - &v.docs, - SExpr::Vec(vec![ - SExpr::word("field"), - v.name.to_sexpr(), - v.tref.to_sexpr(), - ]), - ) + if let Some(ref tref) = v.tref { + SExpr::docs( + &v.docs, + SExpr::Vec(vec![ + SExpr::word("field"), + v.name.to_sexpr(), + tref.to_sexpr(), + ]), + ) + } else { + SExpr::docs( + &v.docs, + SExpr::Vec(vec![SExpr::word("empty"), v.name.to_sexpr()]), + ) + } }) .collect::>(); SExpr::Vec([header, variants].concat()) diff --git a/tools/witx/src/representation.rs b/tools/witx/src/representation.rs index 8ba32a82b..0f12217af 100644 --- a/tools/witx/src/representation.rs +++ b/tools/witx/src/representation.rs @@ -177,7 +177,21 @@ impl Representable for UnionDatatype { } for v in self.variants.iter() { if let Some(byv) = by.variants.iter().find(|byv| byv.name == v.name) { - if v.tref.type_().representable(&*byv.tref.type_()) != RepEquality::Eq { + if v.tref.is_none() && byv.tref.is_none() { + // Both empty is OK + } else if v.tref.is_some() && byv.tref.is_some() { + if v.tref + .as_ref() + .unwrap() + .type_() + .representable(&*byv.tref.as_ref().unwrap().type_()) + != RepEquality::Eq + { + // Fields must be Eq + return RepEquality::NotEq; + } + } else { + // Either one empty means not representable return RepEquality::NotEq; } } else { diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs index f136499fa..e9036411a 100644 --- a/tools/witx/src/validate.rs +++ b/tools/witx/src/validate.rs @@ -3,6 +3,7 @@ use crate::{ parser::{ CommentSyntax, DeclSyntax, Documented, EnumSyntax, FlagsSyntax, HandleSyntax, ImportTypeSyntax, IntSyntax, ModuleDeclSyntax, StructSyntax, TypedefSyntax, UnionSyntax, + VariantSyntax, }, BuiltinType, Definition, Entry, EnumDatatype, EnumVariant, FlagsDatatype, FlagsMember, HandleDatatype, Id, IntConst, IntDatatype, IntRepr, InterfaceFunc, InterfaceFuncParam, @@ -394,30 +395,39 @@ impl DocValidationScope<'_> { let variants = syntax .fields .iter() - .map(|f| { + .map(|v| { + let variant_name = match v.item { + VariantSyntax::Field(ref f) => &f.name, + VariantSyntax::Empty(ref name) => name, + }; let name = variant_scope - .introduce(f.item.name.name(), self.location(f.item.name.span()))?; - let tref = self.validate_datatype(&f.item.type_, false, f.item.name.span())?; - let docs = f.comments.docs(); + .introduce(variant_name.name(), self.location(variant_name.span()))?; + let tref = match &v.item { + VariantSyntax::Field(f) => { + Some(self.validate_datatype(&f.type_, false, variant_name.span())?) + } + VariantSyntax::Empty { .. } => None, + }; + let docs = v.comments.docs(); match variant_name_uses.entry(name.clone()) { hash_map::Entry::Occupied(mut e) => { if *e.get() { Err(ValidationError::InvalidUnionField { - name: f.item.name.name().to_string(), + name: variant_name.name().to_string(), reason: "variant already defined".to_owned(), - location: self.location(f.item.name.span()), + location: self.location(variant_name.span()), })?; } else { e.insert(true); } } hash_map::Entry::Vacant { .. } => Err(ValidationError::InvalidUnionField { - name: f.item.name.name().to_string(), + name: variant_name.name().to_string(), reason: format!( "does not correspond to variant in tag `{}`", tagname.as_str() ), - location: self.location(f.item.name.span()), + location: self.location(variant_name.span()), })?, } Ok(UnionVariant { name, tref, docs }) From a498aa00e7bf6a8f7fcdc01b037523df6092660d Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 5 Feb 2020 11:34:20 -0800 Subject: [PATCH 03/17] unions: add more tests. compute layout. --- tools/witx/src/layout.rs | 57 +++++++++++++++++++++++++-------------- tools/witx/tests/union.rs | 40 +++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 20 deletions(-) diff --git a/tools/witx/src/layout.rs b/tools/witx/src/layout.rs index c89a9cdf0..7c95fc5f1 100644 --- a/tools/witx/src/layout.rs +++ b/tools/witx/src/layout.rs @@ -132,34 +132,51 @@ mod test { } } +pub struct UnionLayout { + pub tag_size: usize, + pub tag_align: usize, + pub contents_offset: usize, + pub contents_size: usize, + pub contents_align: usize, +} + +impl Layout for UnionLayout { + fn mem_size_align(&self) -> SizeAlign { + let align = std::cmp::max(self.tag_align, self.contents_align); + let size = align_to(self.contents_offset + self.contents_size, align); + SizeAlign { size, align } + } +} + impl UnionDatatype { - // TODO also need to expose the layout / existence of the indivdual tag/u members. - fn layout(&self, cache: &mut HashMap) -> SizeAlign { + pub fn union_layout(&self) -> UnionLayout { + let mut cache = HashMap::new(); + self.union_layout_(&mut cache) + } + fn union_layout_(&self, cache: &mut HashMap) -> UnionLayout { let tag = self.tag.layout(cache); - let sas = self + + let variant_sas = self .variants .iter() .filter_map(|v| v.tref.as_ref().map(|t| t.layout(cache))) .collect::>(); - if sas.is_empty() { - tag - } else { - let u_size = sas - .iter() - .map(|sa| sa.size) - .max() - .expect("nonzero variants"); - let u_align = sas - .iter() - .map(|sa| sa.align) - .max() - .expect("nonzero variants"); - let u_offset = align_to(tag.size, u_align); - let align = std::cmp::max(tag.align, u_align); - let size = align_to(u_offset + u_size, align); - SizeAlign { size, align } + + let contents_size = variant_sas.iter().map(|sa| sa.size).max().unwrap_or(0); + let contents_align = variant_sas.iter().map(|sa| sa.align).max().unwrap_or(1); + + UnionLayout { + tag_size: tag.size, + tag_align: tag.align, + contents_offset: align_to(tag.size, contents_align), + contents_size, + contents_align, } } + + fn layout(&self, cache: &mut HashMap) -> SizeAlign { + self.union_layout_(cache).mem_size_align() + } } impl Layout for UnionDatatype { diff --git a/tools/witx/tests/union.rs b/tools/witx/tests/union.rs index 3415302ac..732b5c007 100644 --- a/tools/witx/tests/union.rs +++ b/tools/witx/tests/union.rs @@ -54,6 +54,46 @@ fn two_variant_union() { ); } +#[test] +fn empty_variant_unions() { + let d1 = witx::parse( + "(typename $tag (enum u8 $a $b)) + (typename $u (union $tag (empty $a) (field $b u16)))", + ); + assert!(d1.is_ok(), "d1 is ok"); + + let d2 = witx::parse( + "(typename $tag (enum u8 $a $b)) + (typename $u (union $tag (empty $a) (empty $b)))", + ); + assert!(d2.is_ok(), "d2 is ok"); +} + +#[test] +fn many_variant_unions() { + let d1 = witx::parse( + "(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!(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)))"); From 5abf6ea04c9b26f4a215a747ebca1309b7560c01 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 5 Feb 2020 11:49:55 -0800 Subject: [PATCH 04/17] snapshot 0: translate untagged union syntax to tagged union syntax --- phases/old/snapshot_0/witx/typenames.witx | 41 +++++++---------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/phases/old/snapshot_0/witx/typenames.witx b/phases/old/snapshot_0/witx/typenames.witx index 9a7fc0bb2..13473d66b 100644 --- a/phases/old/snapshot_0/witx/typenames.witx +++ b/phases/old/snapshot_0/witx/typenames.witx @@ -503,8 +503,8 @@ ) ) -;;; The contents of an $event when type is `eventtype::fd_read` or -;;; `eventtype::fd_write`. +;;; The contents of an $event for the `eventtype::fd_read` and +;;; `eventtype::fd_write` variants (typename $event_fd_readwrite (struct ;;; The number of bytes available for reading or writing. @@ -516,9 +516,10 @@ ;;; The contents of an $event. (typename $event_u - (union - ;;; When type is `eventtype::fd_read` or `eventtype::fd_write`: - (field $fd_readwrite $event_fd_readwrite) + (union $eventtype + (field $fd_read $event_fd_readwrite) + (field $fd_write $event_fd_readwrite) + (empty $clock) ) ) @@ -529,9 +530,7 @@ (field $userdata $userdata) ;;; If non-zero, an error that occurred while processing the subscription request. (field $error $errno) - ;;; The type of the event that occurred. - (field $type $eventtype) - ;;; The contents of the event. + ;;; The type of event that occured, and its contents. (field $u $event_u) ) ) @@ -566,7 +565,7 @@ ) ) -;;; The contents of a $subscription when type is type is +;;; The contents of a $subscription when the variant is ;;; `eventtype::fd_read` or `eventtype::fd_write`. (typename $subscription_fd_readwrite (struct @@ -577,11 +576,10 @@ ;;; The contents of a $subscription. (typename $subscription_u - (union - ;;; When type is `eventtype::clock`: + (union $eventtype (field $clock $subscription_clock) - ;;; When type is `eventtype::fd_read` or `eventtype::fd_write`: - (field $fd_readwrite $subscription_fd_readwrite) + (field $fd_read $subscription_fd_readwrite) + (field $fd_write $subscription_fd_readwrite) ) ) @@ -592,8 +590,6 @@ ;;; implementation and returned through `event::userdata`. (field $userdata $userdata) ;;; The type of the event to which to subscribe. - (field $type $eventtype) - ;;; The contents of the subscription. (field $u $subscription_u) ) ) @@ -748,20 +744,9 @@ ) ) -;;; The contents of an $prestat. -(typename $prestat_u - (union - ;;; When type is `PREOPENTYPE_DIR`: - (field $dir $prestat_dir) - ) -) - ;;; Information about a pre-opened capability. (typename $prestat - (struct - ;;; The type of the pre-opened capability. - (field $pr_type $preopentype) - ;;; The contents of the information. - (field $u $prestat_u) + (union $preopentype + (field $dir $prestat_dir) ) ) From 92e976c41f8491adc74dec27048a72b5603c9d6c Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 5 Feb 2020 11:57:01 -0800 Subject: [PATCH 05/17] snapshot: update from untagged union to tagged union syntax --- phases/snapshot/witx/typenames.witx | 34 ++++++++++------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/phases/snapshot/witx/typenames.witx b/phases/snapshot/witx/typenames.witx index ee8c652ec..0fe83e211 100644 --- a/phases/snapshot/witx/typenames.witx +++ b/phases/snapshot/witx/typenames.witx @@ -518,9 +518,10 @@ ;;; The contents of an $event. (typename $event_u - (union - ;;; When type is `eventtype::fd_read` or `eventtype::fd_write`: - (field $fd_readwrite $event_fd_readwrite) + (union $eventtype + (field $fd_read $event_fd_readwrite) + (field $fd_write $event_fd_readwrite) + (empty $clock) ) ) @@ -531,9 +532,7 @@ (field $userdata $userdata) ;;; If non-zero, an error that occurred while processing the subscription request. (field $error $errno) - ;;; The type of the event that occurred. - (field $type $eventtype) - ;;; The contents of the event. + ;;; The type of the event that occurred, and its contents. (field $u $event_u) ) ) @@ -577,11 +576,10 @@ ;;; The contents of a $subscription. (typename $subscription_u - (union - ;;; When type is `eventtype::clock`: + (union $eventtype (field $clock $subscription_clock) - ;;; When type is `eventtype::fd_read` or `eventtype::fd_write`: - (field $fd_readwrite $subscription_fd_readwrite) + (field $fd_read $subscription_fd_readwrite) + (field $fd_write $subscription_fd_readwrite) ) ) @@ -748,20 +746,10 @@ ) ) -;;; The contents of an $prestat. -(typename $prestat_u - (union - ;;; When type is `preopentype::dir`: - (field $dir $prestat_dir) - ) -) - ;;; Information about a pre-opened capability. (typename $prestat - (struct - ;;; The type of the pre-opened capability. - (field $pr_type $preopentype) - ;;; The contents of the information. - (field $u $prestat_u) + (union $preopentype + (field $dir $prestat_dir) ) ) + From c33131e59f6d2aa213ed6e0009558946591ec2bd Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 5 Feb 2020 11:57:33 -0800 Subject: [PATCH 06/17] snapshot: update docs --- phases/snapshot/docs.md | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/phases/snapshot/docs.md b/phases/snapshot/docs.md index 09575008e..4c66df124 100644 --- a/phases/snapshot/docs.md +++ b/phases/snapshot/docs.md @@ -643,8 +643,11 @@ The state of the file descriptor. The contents of an $event. ### Union variants -- `fd_readwrite`: [`event_fd_readwrite`](#event_fd_readwrite) -When type is [`eventtype::fd_read`](#eventtype.fd_read) or [`eventtype::fd_write`](#eventtype.fd_write): +- `fd_read`: [`event_fd_readwrite`](#event_fd_readwrite) + +- `fd_write`: [`event_fd_readwrite`](#event_fd_readwrite) + +- `clock` ## `event`: Struct An event that occurred. @@ -656,11 +659,8 @@ User-provided value that got attached to [`subscription::userdata`](#subscriptio - `error`: [`errno`](#errno) If non-zero, an error that occurred while processing the subscription request. -- `type`: [`eventtype`](#eventtype) -The type of the event that occurred. - - `u`: [`event_u`](#event_u) -The contents of the event. +The type of the event that occurred, and its contents. ## `subclockflags`: Flags(`u16`) Flags determining how to interpret the timestamp provided in @@ -704,10 +704,10 @@ The contents of a $subscription. ### Union variants - `clock`: [`subscription_clock`](#subscription_clock) -When type is [`eventtype::clock`](#eventtype.clock): -- `fd_readwrite`: [`subscription_fd_readwrite`](#subscription_fd_readwrite) -When type is [`eventtype::fd_read`](#eventtype.fd_read) or [`eventtype::fd_write`](#eventtype.fd_write): +- `fd_read`: [`subscription_fd_readwrite`](#subscription_fd_readwrite) + +- `fd_write`: [`subscription_fd_readwrite`](#subscription_fd_readwrite) ## `subscription`: Struct Subscription to an event. @@ -899,22 +899,11 @@ The contents of a $prestat when type is [`preopentype::dir`](#preopentype.dir). - `pr_name_len`: [`size`](#size) The length of the directory name for use with [`fd_prestat_dir_name`](#fd_prestat_dir_name). -## `prestat_u`: Union -The contents of an $prestat. - -### Union variants -- `dir`: [`prestat_dir`](#prestat_dir) -When type is [`preopentype::dir`](#preopentype.dir): - -## `prestat`: Struct +## `prestat`: Union Information about a pre-opened capability. -### Struct members -- `pr_type`: [`preopentype`](#preopentype) -The type of the pre-opened capability. - -- `u`: [`prestat_u`](#prestat_u) -The contents of the information. +### Union variants +- `dir`: [`prestat_dir`](#prestat_dir) # Modules ## wasi_snapshot_preview1 From 4802557080971e69d729931815aabea348ed5213 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 5 Feb 2020 15:37:14 -0800 Subject: [PATCH 07/17] snapshot 0: update docs --- phases/old/snapshot_0/docs.md | 46 ++++++++++++----------------------- 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/phases/old/snapshot_0/docs.md b/phases/old/snapshot_0/docs.md index 360cff906..9b6ffa11d 100644 --- a/phases/old/snapshot_0/docs.md +++ b/phases/old/snapshot_0/docs.md @@ -627,8 +627,8 @@ The state of the file descriptor subscribed to with The peer of this socket has closed or disconnected. ## `event_fd_readwrite`: Struct -The contents of an $event when type is [`eventtype::fd_read`](#eventtype.fd_read) or -[`eventtype::fd_write`](#eventtype.fd_write). +The contents of an $event for the [`eventtype::fd_read`](#eventtype.fd_read) and +[`eventtype::fd_write`](#eventtype.fd_write) variants ### Struct members - `nbytes`: [`filesize`](#filesize) @@ -641,8 +641,11 @@ The state of the file descriptor. The contents of an $event. ### Union variants -- `fd_readwrite`: [`event_fd_readwrite`](#event_fd_readwrite) -When type is [`eventtype::fd_read`](#eventtype.fd_read) or [`eventtype::fd_write`](#eventtype.fd_write): +- `fd_read`: [`event_fd_readwrite`](#event_fd_readwrite) + +- `fd_write`: [`event_fd_readwrite`](#event_fd_readwrite) + +- `clock` ## `event`: Struct An event that occurred. @@ -654,11 +657,8 @@ User-provided value that got attached to [`subscription::userdata`](#subscriptio - `error`: [`errno`](#errno) If non-zero, an error that occurred while processing the subscription request. -- `type`: [`eventtype`](#eventtype) -The type of the event that occurred. - - `u`: [`event_u`](#event_u) -The contents of the event. +The type of event that occured, and its contents. ## `subclockflags`: Flags(`u16`) Flags determining how to interpret the timestamp provided in @@ -693,7 +693,7 @@ to coalesce with other events. Flags specifying whether the timeout is absolute or relative ## `subscription_fd_readwrite`: Struct -The contents of a $subscription when type is type is +The contents of a $subscription when the variant is [`eventtype::fd_read`](#eventtype.fd_read) or [`eventtype::fd_write`](#eventtype.fd_write). ### Struct members @@ -705,10 +705,10 @@ The contents of a $subscription. ### Union variants - `clock`: [`subscription_clock`](#subscription_clock) -When type is [`eventtype::clock`](#eventtype.clock): -- `fd_readwrite`: [`subscription_fd_readwrite`](#subscription_fd_readwrite) -When type is [`eventtype::fd_read`](#eventtype.fd_read) or [`eventtype::fd_write`](#eventtype.fd_write): +- `fd_read`: [`subscription_fd_readwrite`](#subscription_fd_readwrite) + +- `fd_write`: [`subscription_fd_readwrite`](#subscription_fd_readwrite) ## `subscription`: Struct Subscription to an event. @@ -718,11 +718,8 @@ Subscription to an event. User-provided value that is attached to the subscription in the implementation and returned through [`event::userdata`](#event.userdata). -- `type`: [`eventtype`](#eventtype) -The type of the event to which to subscribe. - - `u`: [`subscription_u`](#subscription_u) -The contents of the subscription. +The type of the event to which to subscribe. ## `exitcode`: `u32` Exit code generated by a process when exiting. @@ -900,22 +897,11 @@ The contents of a $prestat when type is `PREOPENTYPE_DIR`. - `pr_name_len`: [`size`](#size) The length of the directory name for use with [`fd_prestat_dir_name`](#fd_prestat_dir_name). -## `prestat_u`: Union -The contents of an $prestat. - -### Union variants -- `dir`: [`prestat_dir`](#prestat_dir) -When type is `PREOPENTYPE_DIR`: - -## `prestat`: Struct +## `prestat`: Union Information about a pre-opened capability. -### Struct members -- `pr_type`: [`preopentype`](#preopentype) -The type of the pre-opened capability. - -- `u`: [`prestat_u`](#prestat_u) -The contents of the information. +### Union variants +- `dir`: [`prestat_dir`](#prestat_dir) # Modules ## wasi_unstable From ff7d19d643dd4a7eba3b18b5105efc55f2a9cf4b Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 5 Feb 2020 15:37:22 -0800 Subject: [PATCH 08/17] ephemeral: translate untagged unions to tagged unions --- phases/ephemeral/witx/typenames.witx | 38 +++++++++------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/phases/ephemeral/witx/typenames.witx b/phases/ephemeral/witx/typenames.witx index 38ca67b1c..a7c55abb2 100644 --- a/phases/ephemeral/witx/typenames.witx +++ b/phases/ephemeral/witx/typenames.witx @@ -560,9 +560,10 @@ ;;; The contents of an $event. (typename $event_u - (union - ;;; When type is `eventtype::fd_read` or `eventtype::fd_write`: - (field $fd_readwrite $event_fd_readwrite) + (union $eventtype + (field $fd_read $event_fd_readwrite) + (field $fd_write $event_fd_readwrite) + (empty $clock) ) ) @@ -573,9 +574,7 @@ (field $userdata $userdata) ;;; If non-zero, an error that occurred while processing the subscription request. (field $error $errno) - ;;; The type of the event that occurred. - (field $type $eventtype) - ;;; The contents of the event. + ;;; The type of the event that occurred, and the contents of the event (field $u $event_u) ) ) @@ -619,11 +618,10 @@ ;;; The contents of a $subscription. (typename $subscription_u - (union - ;;; When type is `eventtype::clock`: + (union $eventtype (field $clock $subscription_clock) - ;;; When type is `eventtype::fd_read` or `eventtype::fd_write`: - (field $fd_readwrite $subscription_fd_readwrite) + (field $fd_read $subscription_fd_readwrite) + (field $fd_write $subscription_fd_readwrite) ) ) @@ -633,9 +631,7 @@ ;;; User-provided value that is attached to the subscription in the ;;; implementation and returned through `event::userdata`. (field $userdata $userdata) - ;;; The type of the event to which to subscribe. - (field $type $eventtype) - ;;; The contents of the subscription. + ;;; The type of the event to which to subscribe, and the contents of the subscription. (field $u $subscription_u) ) ) @@ -691,20 +687,10 @@ ) ) -;;; The contents of an $prestat. -(typename $prestat_u - (union - ;;; When $pr_type of the $prestat is `preopentype::dir`: - (field $dir $prestat_dir) - ) -) - ;;; Information about a pre-opened capability. (typename $prestat - (struct - ;;; The type of the pre-opened capability. - (field $pr_type $preopentype) - ;;; The contents of the information. - (field $u $prestat_u) + (union $preopentype + ;;; When type is `preopentype::dir`: + (field $dir $prestat_dir) ) ) From 123775a9a0fa9f2d4ba94fbfaa37334ba9d72fdc Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 5 Feb 2020 16:30:39 -0800 Subject: [PATCH 09/17] render: fix union output --- tools/witx/src/render.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/witx/src/render.rs b/tools/witx/src/render.rs index 6bb6e290e..fc775032d 100644 --- a/tools/witx/src/render.rs +++ b/tools/witx/src/render.rs @@ -206,7 +206,11 @@ impl StructDatatype { impl UnionDatatype { pub fn to_sexpr(&self) -> SExpr { - let header = vec![SExpr::word("union")]; + let tag_name = match self.tag { + TypeRef::Name(ref tn) => &tn.name, + TypeRef::Value { .. } => unreachable!("union tags must be named types"), + }; + let header = vec![SExpr::word("union"), tag_name.to_sexpr()]; let variants = self .variants .iter() From 2151684ed8f1236bd1d09fe2c51a867d65623b29 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 5 Feb 2020 16:33:43 -0800 Subject: [PATCH 10/17] bump witx to 0.8.0 - this is definitely semver breaking --- tools/witx/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/witx/Cargo.toml b/tools/witx/Cargo.toml index 959c22143..32f0cbea9 100644 --- a/tools/witx/Cargo.toml +++ b/tools/witx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "witx" -version = "0.7.0" +version = "0.8.0" description = "Parse and validate witx file format" homepage = "https://github.com/WebAssembly/WASI" repository = "https://github.com/WebAssembly/WASI" From 15e848c2d10ca91fb37e759e5678d785bfb7e12a Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 6 Feb 2020 13:03:46 -0800 Subject: [PATCH 11/17] snapshot: I forgot to delete the tag field `subscription.type` --- phases/snapshot/witx/typenames.witx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/phases/snapshot/witx/typenames.witx b/phases/snapshot/witx/typenames.witx index 0fe83e211..812281f3c 100644 --- a/phases/snapshot/witx/typenames.witx +++ b/phases/snapshot/witx/typenames.witx @@ -589,9 +589,7 @@ ;;; User-provided value that is attached to the subscription in the ;;; implementation and returned through `event::userdata`. (field $userdata $userdata) - ;;; The type of the event to which to subscribe. - (field $type $eventtype) - ;;; The contents of the subscription. + ;;; The type of the event to which to subscribe, and its contents (field $u $subscription_u) ) ) From 2146fa905e652524be3de4633cbb95f6c865be4f Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 6 Feb 2020 18:15:29 -0800 Subject: [PATCH 12/17] regen docs --- phases/snapshot/docs.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/phases/snapshot/docs.md b/phases/snapshot/docs.md index 4c66df124..3dc969f89 100644 --- a/phases/snapshot/docs.md +++ b/phases/snapshot/docs.md @@ -717,11 +717,8 @@ Subscription to an event. User-provided value that is attached to the subscription in the implementation and returned through [`event::userdata`](#event.userdata). -- `type`: [`eventtype`](#eventtype) -The type of the event to which to subscribe. - - `u`: [`subscription_u`](#subscription_u) -The contents of the subscription. +The type of the event to which to subscribe, and its contents ## `exitcode`: `u32` Exit code generated by a process when exiting. From 16c122f1f0836af9bdc4b4f67a20b6346d0ef3e6 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 7 Feb 2020 14:59:31 -0800 Subject: [PATCH 13/17] unionlayout: derive the usual suspects --- tools/witx/src/layout.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/witx/src/layout.rs b/tools/witx/src/layout.rs index 7c95fc5f1..62d93f404 100644 --- a/tools/witx/src/layout.rs +++ b/tools/witx/src/layout.rs @@ -132,6 +132,7 @@ mod test { } } +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct UnionLayout { pub tag_size: usize, pub tag_align: usize, From a83f0a721ca145147f25f6af51bf2fedf77b5d83 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 12 Feb 2020 09:36:23 -0800 Subject: [PATCH 14/17] snapshot 1 and 0: put tag back in event struct, flatten event_u prior to this PR, event_u was a union with only one variant fd_readwrite. the change into tagged union ended up changing the memory layout of the event struct. so, rather than use tagged unions in the event struct, we just use a struct that you ignore in the case of the clock event, which has the same ABI as previously. in ephemeral, we use a tagged union for events, because we dont have any ABI stability guarantees to maintain there. --- phases/old/snapshot_0/docs.md | 18 ++++++------------ phases/old/snapshot_0/witx/typenames.witx | 16 +++++----------- phases/snapshot/docs.md | 18 ++++++------------ phases/snapshot/witx/typenames.witx | 16 +++++----------- 4 files changed, 22 insertions(+), 46 deletions(-) diff --git a/phases/old/snapshot_0/docs.md b/phases/old/snapshot_0/docs.md index 9b6ffa11d..3466d13ad 100644 --- a/phases/old/snapshot_0/docs.md +++ b/phases/old/snapshot_0/docs.md @@ -637,16 +637,6 @@ The number of bytes available for reading or writing. - `flags`: [`eventrwflags`](#eventrwflags) The state of the file descriptor. -## `event_u`: Union -The contents of an $event. - -### Union variants -- `fd_read`: [`event_fd_readwrite`](#event_fd_readwrite) - -- `fd_write`: [`event_fd_readwrite`](#event_fd_readwrite) - -- `clock` - ## `event`: Struct An event that occurred. @@ -657,8 +647,12 @@ User-provided value that got attached to [`subscription::userdata`](#subscriptio - `error`: [`errno`](#errno) If non-zero, an error that occurred while processing the subscription request. -- `u`: [`event_u`](#event_u) -The type of event that occured, and its contents. +- `type`: [`eventtype`](#eventtype) +The type of event that occured + +- `fd_readwrite`: [`event_fd_readwrite`](#event_fd_readwrite) +The contents of the event, if it is an [`eventtype::fd_read`](#eventtype.fd_read) or +[`eventtype::fd_write`](#eventtype.fd_write). [`eventtype::clock`](#eventtype.clock) events ignore this field. ## `subclockflags`: Flags(`u16`) Flags determining how to interpret the timestamp provided in diff --git a/phases/old/snapshot_0/witx/typenames.witx b/phases/old/snapshot_0/witx/typenames.witx index 13473d66b..493bda050 100644 --- a/phases/old/snapshot_0/witx/typenames.witx +++ b/phases/old/snapshot_0/witx/typenames.witx @@ -514,15 +514,6 @@ ) ) -;;; The contents of an $event. -(typename $event_u - (union $eventtype - (field $fd_read $event_fd_readwrite) - (field $fd_write $event_fd_readwrite) - (empty $clock) - ) -) - ;;; An event that occurred. (typename $event (struct @@ -530,8 +521,11 @@ (field $userdata $userdata) ;;; If non-zero, an error that occurred while processing the subscription request. (field $error $errno) - ;;; The type of event that occured, and its contents. - (field $u $event_u) + ;;; The type of event that occured + (field $type $eventtype) + ;;; The contents of the event, if it is an `eventtype::fd_read` or + ;;; `eventtype::fd_write`. `eventtype::clock` events ignore this field. + (field $fd_readwrite $event_fd_readwrite) ) ) diff --git a/phases/snapshot/docs.md b/phases/snapshot/docs.md index 3dc969f89..0b3dec378 100644 --- a/phases/snapshot/docs.md +++ b/phases/snapshot/docs.md @@ -639,16 +639,6 @@ The number of bytes available for reading or writing. - `flags`: [`eventrwflags`](#eventrwflags) The state of the file descriptor. -## `event_u`: Union -The contents of an $event. - -### Union variants -- `fd_read`: [`event_fd_readwrite`](#event_fd_readwrite) - -- `fd_write`: [`event_fd_readwrite`](#event_fd_readwrite) - -- `clock` - ## `event`: Struct An event that occurred. @@ -659,8 +649,12 @@ User-provided value that got attached to [`subscription::userdata`](#subscriptio - `error`: [`errno`](#errno) If non-zero, an error that occurred while processing the subscription request. -- `u`: [`event_u`](#event_u) -The type of the event that occurred, and its contents. +- `type`: [`eventtype`](#eventtype) +The type of event that occured + +- `fd_readwrite`: [`event_fd_readwrite`](#event_fd_readwrite) +The contents of the event, if it is an [`eventtype::fd_read`](#eventtype.fd_read) or +[`eventtype::fd_write`](#eventtype.fd_write). [`eventtype::clock`](#eventtype.clock) events ignore this field. ## `subclockflags`: Flags(`u16`) Flags determining how to interpret the timestamp provided in diff --git a/phases/snapshot/witx/typenames.witx b/phases/snapshot/witx/typenames.witx index 812281f3c..eb8cb8c64 100644 --- a/phases/snapshot/witx/typenames.witx +++ b/phases/snapshot/witx/typenames.witx @@ -516,15 +516,6 @@ ) ) -;;; The contents of an $event. -(typename $event_u - (union $eventtype - (field $fd_read $event_fd_readwrite) - (field $fd_write $event_fd_readwrite) - (empty $clock) - ) -) - ;;; An event that occurred. (typename $event (struct @@ -532,8 +523,11 @@ (field $userdata $userdata) ;;; If non-zero, an error that occurred while processing the subscription request. (field $error $errno) - ;;; The type of the event that occurred, and its contents. - (field $u $event_u) + ;;; The type of event that occured + (field $type $eventtype) + ;;; The contents of the event, if it is an `eventtype::fd_read` or + ;;; `eventtype::fd_write`. `eventtype::clock` events ignore this field. + (field $fd_readwrite $event_fd_readwrite) ) ) From dbb8f5e457c355e7732d14c85e76fb64d581c2d4 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 13 Feb 2020 15:24:58 -0800 Subject: [PATCH 15/17] layout: refactor so we have impls for NamedType and Type too --- tools/witx/src/layout.rs | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/tools/witx/src/layout.rs b/tools/witx/src/layout.rs index 62d93f404..3e2f6ee09 100644 --- a/tools/witx/src/layout.rs +++ b/tools/witx/src/layout.rs @@ -22,7 +22,37 @@ impl TypeRef { if let Some(hit) = cache.get(self) { return *hit; } - let layout = match &*self.type_() { + let layout = match &self { + TypeRef::Name(nt) => nt.layout(cache), + TypeRef::Value(v) => v.layout(cache), + }; + cache.insert(self.clone(), layout); + layout + } +} + +impl Layout for TypeRef { + fn mem_size_align(&self) -> SizeAlign { + let mut cache = HashMap::new(); + self.layout(&mut cache) + } +} + +impl NamedType { + fn layout(&self, cache: &mut HashMap) -> SizeAlign { + self.tref.layout(cache) + } +} +impl Layout for NamedType { + fn mem_size_align(&self) -> SizeAlign { + let mut cache = HashMap::new(); + self.layout(&mut cache) + } +} + +impl Type { + fn layout(&self, cache: &mut HashMap) -> SizeAlign { + match &self { Type::Enum(e) => e.repr.mem_size_align(), Type::Int(i) => i.repr.mem_size_align(), Type::Flags(f) => f.repr.mem_size_align(), @@ -32,18 +62,17 @@ impl TypeRef { Type::Array { .. } => BuiltinType::String.mem_size_align(), Type::Pointer { .. } | Type::ConstPointer { .. } => BuiltinType::U32.mem_size_align(), Type::Builtin(b) => b.mem_size_align(), - }; - cache.insert(self.clone(), layout); - layout + } } } -impl Layout for TypeRef { +impl Layout for Type { fn mem_size_align(&self) -> SizeAlign { let mut cache = HashMap::new(); self.layout(&mut cache) } } + impl Layout for IntRepr { fn mem_size_align(&self) -> SizeAlign { match self { From 54db80fa5ff0060a7f95fe21d62a1d53cc846721 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 13 Feb 2020 16:26:24 -0800 Subject: [PATCH 16/17] ast: change tag of UnionDatatype to be a NamedType this is much more convenient for consumers, and it reflects the requirement of the syntax to provide an Id --- tools/witx/src/ast.rs | 2 +- tools/witx/src/render.rs | 6 +----- tools/witx/src/validate.rs | 9 ++++----- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index 31dd6246e..564d4ea1d 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -247,7 +247,7 @@ pub struct StructMember { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct UnionDatatype { - pub tag: TypeRef, + pub tag: Rc, pub variants: Vec, } diff --git a/tools/witx/src/render.rs b/tools/witx/src/render.rs index fc775032d..64b429f8b 100644 --- a/tools/witx/src/render.rs +++ b/tools/witx/src/render.rs @@ -206,11 +206,7 @@ impl StructDatatype { impl UnionDatatype { pub fn to_sexpr(&self) -> SExpr { - let tag_name = match self.tag { - TypeRef::Name(ref tn) => &tn.name, - TypeRef::Value { .. } => unreachable!("union tags must be named types"), - }; - let header = vec![SExpr::word("union"), tag_name.to_sexpr()]; + let header = vec![SExpr::word("union"), self.tag.name.to_sexpr()]; let variants = self .variants .iter() diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs index e9036411a..17be21e82 100644 --- a/tools/witx/src/validate.rs +++ b/tools/witx/src/validate.rs @@ -360,7 +360,7 @@ impl DocValidationScope<'_> { ) -> Result { let mut variant_scope = IdentValidation::new(); let tag_id = self.get(&syntax.tag)?; - let (tag, tagname, mut variant_name_uses) = match self.doc.entries.get(&tag_id) { + let (tag, mut variant_name_uses) = match self.doc.entries.get(&tag_id) { Some(Entry::Typename(weak_ref)) => { let named_dt = weak_ref.upgrade().expect("weak backref to defined type"); match &*named_dt.type_() { @@ -370,8 +370,7 @@ impl DocValidationScope<'_> { .iter() .map(|v| (v.name.clone(), false)) .collect::>(); - let name = named_dt.name.clone(); - Ok((TypeRef::Name(named_dt), name, uses)) + Ok((named_dt, uses)) } other => Err(ValidationError::WrongKindName { name: syntax.tag.name().to_string(), @@ -425,7 +424,7 @@ impl DocValidationScope<'_> { name: variant_name.name().to_string(), reason: format!( "does not correspond to variant in tag `{}`", - tagname.as_str() + tag.name.as_str() ), location: self.location(variant_name.span()), })?, @@ -446,7 +445,7 @@ impl DocValidationScope<'_> { .map(|i| i.as_str()) .collect::>() .join(", "), - reason: format!("missing variants from tag `{}`", tagname.as_str()), + reason: format!("missing variants from tag `{}`", tag.name.as_str()), location: self.location(span), })?; } From 7219c5a34470b04d033a8635c014f835d1b32c02 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 13 Feb 2020 17:12:17 -0800 Subject: [PATCH 17/17] fixup docs --- phases/ephemeral/docs.md | 41 ++++++++++++++-------------------------- 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/phases/ephemeral/docs.md b/phases/ephemeral/docs.md index fed63a44e..a046e9ce8 100644 --- a/phases/ephemeral/docs.md +++ b/phases/ephemeral/docs.md @@ -687,8 +687,11 @@ The state of the file descriptor. The contents of an $event. ### Union variants -- `fd_readwrite`: [`event_fd_readwrite`](#event_fd_readwrite) -When type is [`eventtype::fd_read`](#eventtype.fd_read) or [`eventtype::fd_write`](#eventtype.fd_write): +- `fd_read`: [`event_fd_readwrite`](#event_fd_readwrite) + +- `fd_write`: [`event_fd_readwrite`](#event_fd_readwrite) + +- `clock` ## `event`: Struct An event that occurred. @@ -700,11 +703,8 @@ User-provided value that got attached to [`subscription::userdata`](#subscriptio - `error`: [`errno`](#errno) If non-zero, an error that occurred while processing the subscription request. -- `type`: [`eventtype`](#eventtype) -The type of the event that occurred. - - `u`: [`event_u`](#event_u) -The contents of the event. +The type of the event that occurred, and the contents of the event ## `subclockflags`: Flags(`u16`) Flags determining how to interpret the timestamp provided in @@ -748,10 +748,10 @@ The contents of a $subscription. ### Union variants - `clock`: [`subscription_clock`](#subscription_clock) -When type is [`eventtype::clock`](#eventtype.clock): -- `fd_readwrite`: [`subscription_fd_readwrite`](#subscription_fd_readwrite) -When type is [`eventtype::fd_read`](#eventtype.fd_read) or [`eventtype::fd_write`](#eventtype.fd_write): +- `fd_read`: [`subscription_fd_readwrite`](#subscription_fd_readwrite) + +- `fd_write`: [`subscription_fd_readwrite`](#subscription_fd_readwrite) ## `subscription`: Struct Subscription to an event. @@ -761,11 +761,8 @@ Subscription to an event. User-provided value that is attached to the subscription in the implementation and returned through [`event::userdata`](#event.userdata). -- `type`: [`eventtype`](#eventtype) -The type of the event to which to subscribe. - - `u`: [`subscription_u`](#subscription_u) -The contents of the subscription. +The type of the event to which to subscribe, and the contents of the subscription. ## `exitcode`: `u32` Exit code generated by a process when exiting. @@ -815,22 +812,12 @@ The contents of a $prestat when its $pr_type is [`preopentype::dir`](#preopentyp - `pr_name_len`: [`size`](#size) The length of the directory name for use with `fd_prestat_dir_name`. -## `prestat_u`: Union -The contents of an $prestat. - -### Union variants -- `dir`: [`prestat_dir`](#prestat_dir) -When $pr_type of the $prestat is [`preopentype::dir`](#preopentype.dir): - -## `prestat`: Struct +## `prestat`: Union Information about a pre-opened capability. -### Struct members -- `pr_type`: [`preopentype`](#preopentype) -The type of the pre-opened capability. - -- `u`: [`prestat_u`](#prestat_u) -The contents of the information. +### Union variants +- `dir`: [`prestat_dir`](#prestat_dir) +When type is [`preopentype::dir`](#preopentype.dir): # Modules ## wasi_ephemeral_args