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 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) ) ) diff --git a/phases/old/snapshot_0/docs.md b/phases/old/snapshot_0/docs.md index 360cff906..3466d13ad 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) @@ -637,13 +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_readwrite`: [`event_fd_readwrite`](#event_fd_readwrite) -When type is [`eventtype::fd_read`](#eventtype.fd_read) or [`eventtype::fd_write`](#eventtype.fd_write): - ## `event`: Struct An event that occurred. @@ -655,10 +648,11 @@ User-provided value that got attached to [`subscription::userdata`](#subscriptio If non-zero, an error that occurred while processing the subscription request. - `type`: [`eventtype`](#eventtype) -The type of the event that occurred. +The type of event that occured -- `u`: [`event_u`](#event_u) -The contents of the event. +- `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 @@ -693,7 +687,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 +699,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 +712,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 +891,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 diff --git a/phases/old/snapshot_0/witx/typenames.witx b/phases/old/snapshot_0/witx/typenames.witx index 9a7fc0bb2..493bda050 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. @@ -514,14 +514,6 @@ ) ) -;;; 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) - ) -) - ;;; An event that occurred. (typename $event (struct @@ -529,10 +521,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. + ;;; The type of event that occured (field $type $eventtype) - ;;; The contents of the event. - (field $u $event_u) + ;;; 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) ) ) @@ -566,7 +559,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 +570,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 +584,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 +738,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) ) ) diff --git a/phases/snapshot/docs.md b/phases/snapshot/docs.md index 09575008e..0b3dec378 100644 --- a/phases/snapshot/docs.md +++ b/phases/snapshot/docs.md @@ -639,13 +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_readwrite`: [`event_fd_readwrite`](#event_fd_readwrite) -When type is [`eventtype::fd_read`](#eventtype.fd_read) or [`eventtype::fd_write`](#eventtype.fd_write): - ## `event`: Struct An event that occurred. @@ -657,10 +650,11 @@ User-provided value that got attached to [`subscription::userdata`](#subscriptio If non-zero, an error that occurred while processing the subscription request. - `type`: [`eventtype`](#eventtype) -The type of the event that occurred. +The type of event that occured -- `u`: [`event_u`](#event_u) -The contents of the event. +- `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 @@ -704,10 +698,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. @@ -717,11 +711,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. @@ -899,22 +890,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 diff --git a/phases/snapshot/witx/typenames.witx b/phases/snapshot/witx/typenames.witx index ee8c652ec..eb8cb8c64 100644 --- a/phases/snapshot/witx/typenames.witx +++ b/phases/snapshot/witx/typenames.witx @@ -516,14 +516,6 @@ ) ) -;;; 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) - ) -) - ;;; An event that occurred. (typename $event (struct @@ -531,10 +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. + ;;; The type of event that occured (field $type $eventtype) - ;;; The contents of the event. - (field $u $event_u) + ;;; 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) ) ) @@ -577,11 +570,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) ) ) @@ -591,9 +583,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) ) ) @@ -748,20 +738,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) ) ) + 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" diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index fcb33ad44..564d4ea1d 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -247,13 +247,14 @@ pub struct StructMember { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct UnionDatatype { + pub tag: Rc, pub variants: Vec, } #[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..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 { @@ -132,25 +161,51 @@ mod test { } } +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +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 { - fn layout(&self, cache: &mut HashMap) -> SizeAlign { - let sas = self + 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 variant_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 } + + 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() } } diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs index 08bc9af5b..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,20 +478,48 @@ 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 fields: Vec>>, + 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/render.rs b/tools/witx/src/render.rs index a1a6ff58a..64b429f8b 100644 --- a/tools/witx/src/render.rs +++ b/tools/witx/src/render.rs @@ -206,19 +206,26 @@ impl StructDatatype { impl UnionDatatype { pub fn to_sexpr(&self) -> SExpr { - let header = vec![SExpr::word("union")]; + let header = vec![SExpr::word("union"), self.tag.name.to_sexpr()]; let variants = self .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 3bdd6a792..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 { @@ -185,9 +199,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 +311,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..17be21e82 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, @@ -10,7 +11,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 +44,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 +61,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,22 +356,100 @@ 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, 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::>(); + Ok((named_dt, 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() - .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: variant_name.name().to_string(), + reason: "variant already defined".to_owned(), + location: self.location(variant_name.span()), + })?; + } else { + e.insert(true); + } + } + hash_map::Entry::Vacant { .. } => Err(ValidationError::InvalidUnionField { + name: variant_name.name().to_string(), + reason: format!( + "does not correspond to variant in tag `{}`", + tag.name.as_str() + ), + location: self.location(variant_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 `{}`", tag.name.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..732b5c007 --- /dev/null +++ b/tools/witx/tests/union.rs @@ -0,0 +1,183 @@ +// 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 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)))"); + 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 + } + } +}