From 436d1813bb1635e0170e43c5f2e436ff87f5deea Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 28 Apr 2020 15:16:03 -0700 Subject: [PATCH 1/8] witx: add profiles to the ast and parser --- tools/witx/examples/witx.rs | 25 +++++ tools/witx/src/ast.rs | 129 ++++++++++++++++++++++ tools/witx/src/lib.rs | 4 +- tools/witx/src/parser.rs | 46 +++++++- tools/witx/src/render.rs | 37 ++++++- tools/witx/src/validate.rs | 215 +++++++++++++++++++++++++----------- tools/witx/tests/wasi.rs | 4 + 7 files changed, 391 insertions(+), 69 deletions(-) diff --git a/tools/witx/examples/witx.rs b/tools/witx/examples/witx.rs index f86140ab8..87c558e99 100644 --- a/tools/witx/examples/witx.rs +++ b/tools/witx/examples/witx.rs @@ -40,6 +40,20 @@ enum Command { )] output: Option, }, + /// Parse and then render document as s-exprs - should be equivelant, modulo whitespace. + Render { + /// Path to root of witx document + #[structopt(number_of_values = 1, value_name = "INPUT", parse(from_os_str))] + input: Vec, + /// Path to generated documentation in Markdown format + #[structopt( + short = "o", + long = "output", + value_name = "OUTPUT", + parse(from_os_str) + )] + output: Option, + }, /// Update documentation in WASI repository to reflect witx specs RepoDocs, /// Examine differences between interfaces @@ -88,6 +102,17 @@ pub fn main() { println!("{}", doc.to_md()) } } + Command::Render { input, output } => { + let doc = load_witx(&input, "input", verbose); + let rendered = format!("{}", doc); + if let Some(output) = output { + let mut file = File::create(output).expect("create output file"); + file.write_all(rendered.as_str().as_bytes()) + .expect("write output file"); + } else { + println!("{}", rendered); + } + } Command::RepoDocs => { for phase in &[ phases::snapshot().unwrap(), diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index 661e91475..99166f87f 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -63,6 +63,18 @@ impl Document { _ => None, }) } + pub fn profile(&self, name: &Id) -> Option> { + self.entries.get(&name).and_then(|e| match e { + Entry::Profile(m) => Some(m.upgrade().expect("always possible to upgrade entry")), + _ => None, + }) + } + pub fn profiles<'a>(&'a self) -> impl Iterator> + 'a { + self.definitions.iter().filter_map(|d| match d { + Definition::Profile(m) => Some(m.clone()), + _ => None, + }) + } } impl PartialEq for Document { @@ -84,12 +96,14 @@ impl std::hash::Hash for Document { pub enum Definition { Typename(Rc), Module(Rc), + Profile(Rc), } #[derive(Debug, Clone)] pub enum Entry { Typename(Weak), Module(Weak), + Profile(Weak), } impl Entry { @@ -97,6 +111,7 @@ impl Entry { match self { Entry::Typename { .. } => "typename", Entry::Module { .. } => "module", + Entry::Profile { .. } => "profile", } } } @@ -118,6 +133,13 @@ impl PartialEq for Entry { .upgrade() .expect("possible to upgrade entry when part of document") } + (Entry::Profile(p), Entry::Profile(p_rhs)) => { + p.upgrade() + .expect("possible to upgrade entry when part of document") + == p_rhs + .upgrade() + .expect("possible to upgrade entry when part of document") + } _ => false, } } @@ -408,3 +430,110 @@ pub enum InterfaceFuncParamPosition { Param(usize), Result(usize), } + +#[derive(Debug, Clone)] +pub struct Profile { + pub name: Id, + definitions: Vec, + entries: HashMap, + pub docs: String, +} + +impl Profile { + pub fn new( + name: Id, + definitions: Vec, + entries: HashMap, + docs: String, + ) -> Self { + Self { + name, + definitions, + entries, + docs, + } + } + pub fn import(&self, name: &Id) -> Option> { + self.entries.get(name).and_then(|e| match e { + ProfileEntry::Import(weak) => { + Some(weak.upgrade().expect("always possible to upgrade entry")) + } + _ => None, + }) + } + pub fn imports<'a>(&'a self) -> impl Iterator> + 'a { + self.definitions.iter().filter_map(|d| match d { + ProfileDefinition::Import(def) => Some(def.clone()), + _ => None, + }) + } + pub fn func(&self, name: &Id) -> Option> { + self.entries.get(name).and_then(|e| match e { + ProfileEntry::Func(weak) => { + Some(weak.upgrade().expect("always possible to upgrade entry")) + } + _ => None, + }) + } + pub fn funcs<'a>(&'a self) -> impl Iterator> + 'a { + self.definitions.iter().filter_map(|d| match d { + ProfileDefinition::Func(def) => Some(def.clone()), + _ => None, + }) + } +} + +impl PartialEq for Profile { + fn eq(&self, rhs: &Profile) -> bool { + // For equality, we don't care about the ordering of definitions, + // so we only need to check that the entries map is equal + self.entries == rhs.entries + } +} +impl Eq for Profile {} + +impl std::hash::Hash for Profile { + fn hash(&self, state: &mut H) { + std::hash::Hash::hash(&self.definitions, state); + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct ProfileImport { + pub module: Rc, + pub docs: String, +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum ProfileDefinition { + Import(Rc), + Func(Rc), +} + +#[derive(Debug, Clone)] +pub enum ProfileEntry { + Import(Weak), + Func(Weak), +} + +impl PartialEq for ProfileEntry { + fn eq(&self, rhs: &ProfileEntry) -> bool { + match (self, rhs) { + (ProfileEntry::Import(i), ProfileEntry::Import(i_rhs)) => { + i.upgrade() + .expect("always possible to upgrade profileentry when part of profile") + == i_rhs + .upgrade() + .expect("always possible to upgrade profileentry when part of profile") + } + (ProfileEntry::Func(i), ProfileEntry::Func(i_rhs)) => { + i.upgrade() + .expect("always possible to upgrade profileentry when part of profile") + == i_rhs + .upgrade() + .expect("always possible to upgrade profileentry when part of profile") + } + _ => false, + } + } +} diff --git a/tools/witx/src/lib.rs b/tools/witx/src/lib.rs index c612e601d..928f4afbd 100644 --- a/tools/witx/src/lib.rs +++ b/tools/witx/src/lib.rs @@ -27,8 +27,8 @@ pub use ast::{ BuiltinType, Definition, Document, Entry, EnumDatatype, EnumVariant, FlagsDatatype, FlagsMember, HandleDatatype, Id, IntConst, IntDatatype, IntRepr, InterfaceFunc, InterfaceFuncParam, InterfaceFuncParamPosition, Module, ModuleDefinition, ModuleEntry, - ModuleImport, ModuleImportVariant, NamedType, StructDatatype, StructMember, Type, TypeRef, - UnionDatatype, UnionVariant, + ModuleImport, ModuleImportVariant, NamedType, Profile, ProfileDefinition, ProfileEntry, + ProfileImport, StructDatatype, StructMember, Type, TypeRef, UnionDatatype, UnionVariant, }; pub use coretypes::{AtomType, CoreFuncType, CoreParamSignifies, CoreParamType, TypePassedBy}; pub use docs::Documentation; diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs index be0cb31e1..1ac5f4e46 100644 --- a/tools/witx/src/parser.rs +++ b/tools/witx/src/parser.rs @@ -46,6 +46,7 @@ mod kw { wast::custom_keyword!(u64); wast::custom_keyword!(u8); wast::custom_keyword!(usize); + wast::custom_keyword!(profile); } mod annotation { @@ -237,6 +238,7 @@ impl<'a> Parse<'a> for TopLevelSyntax<'a> { pub enum DeclSyntax<'a> { Typename(TypenameSyntax<'a>), Module(ModuleSyntax<'a>), + Profile(ProfileSyntax<'a>), } impl<'a> Parse<'a> for DeclSyntax<'a> { @@ -246,6 +248,8 @@ impl<'a> Parse<'a> for DeclSyntax<'a> { Ok(DeclSyntax::Module(parser.parse()?)) } else if l.peek::() { Ok(DeclSyntax::Typename(parser.parse()?)) + } else if l.peek::() { + Ok(DeclSyntax::Profile(parser.parse()?)) } else { Err(l.error()) } @@ -529,6 +533,7 @@ impl<'a> Parse<'a> for ModuleDeclSyntax<'a> { parser.parens(|p| { let mut l = p.lookahead1(); if l.peek::() { + let _ = parser.parse::().expect("just peeked it"); Ok(ModuleDeclSyntax::Import(p.parse()?)) } else if l.peek::() { Ok(ModuleDeclSyntax::Func(p.parse()?)) @@ -548,7 +553,6 @@ pub struct ModuleImportSyntax<'a> { impl<'a> Parse<'a> for ModuleImportSyntax<'a> { fn parse(parser: Parser<'a>) -> Result { - parser.parse::()?; let name_loc = parser.cur_span(); Ok(ModuleImportSyntax { name: parser.parse()?, @@ -681,3 +685,43 @@ impl PartialEq for InterfaceFuncSyntax<'_> { } impl Eq for InterfaceFuncSyntax<'_> {} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ProfileSyntax<'a> { + pub name: wast::Id<'a>, + pub decls: Vec>>, +} + +impl<'a> Parse<'a> for ProfileSyntax<'a> { + fn parse(parser: Parser<'a>) -> Result { + parser.parse::()?; + let name = parser.parse()?; + let mut decls = Vec::new(); + while !parser.is_empty() { + decls.push(parser.parse()?); + } + Ok(ProfileSyntax { name, decls }) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ProfileDeclSyntax<'a> { + Import(wast::Id<'a>), + Func(InterfaceFuncSyntax<'a>), +} + +impl<'a> Parse<'a> for ProfileDeclSyntax<'a> { + fn parse(parser: Parser<'a>) -> Result { + parser.parens(|p| { + let mut l = p.lookahead1(); + if l.peek::() { + let _ = p.parse::().expect("just peeked kw"); + Ok(ProfileDeclSyntax::Import(p.parse()?)) + } else if l.peek::() { + Ok(ProfileDeclSyntax::Func(p.parse()?)) + } else { + Err(l.error()) + } + }) + } +} diff --git a/tools/witx/src/render.rs b/tools/witx/src/render.rs index 64b429f8b..dea2718de 100644 --- a/tools/witx/src/render.rs +++ b/tools/witx/src/render.rs @@ -9,6 +9,9 @@ impl fmt::Display for Document { for m in self.modules() { write!(f, "{}\n", m.to_sexpr())?; } + for p in self.profiles() { + write!(f, "{}\n", p.to_sexpr())?; + } Ok(()) } } @@ -266,17 +269,22 @@ impl Module { } } -impl ModuleImport { +impl ModuleImportVariant { pub fn to_sexpr(&self) -> SExpr { - let variant = match self.variant { + match self { ModuleImportVariant::Memory => SExpr::Vec(vec![SExpr::word("memory")]), - }; + } + } +} + +impl ModuleImport { + pub fn to_sexpr(&self) -> SExpr { SExpr::docs( &self.docs, SExpr::Vec(vec![ SExpr::word("import"), SExpr::quote(self.name.as_str()), - variant, + self.variant.to_sexpr(), ]), ) } @@ -331,3 +339,24 @@ impl InterfaceFunc { ) } } + +impl Profile { + pub fn to_sexpr(&self) -> SExpr { + let header = vec![SExpr::word("profile"), self.name.to_sexpr()]; + let definitions = self + .imports() + .map(|i| i.to_sexpr()) + .chain(self.funcs().map(|f| f.to_sexpr())) + .collect::>(); + SExpr::docs(&self.docs, SExpr::Vec([header, definitions].concat())) + } +} + +impl ProfileImport { + pub fn to_sexpr(&self) -> SExpr { + SExpr::docs( + &self.docs, + SExpr::Vec(vec![SExpr::word("import"), self.module.name.to_sexpr()]), + ) + } +} diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs index 17be21e82..35eab4a25 100644 --- a/tools/witx/src/validate.rs +++ b/tools/witx/src/validate.rs @@ -2,14 +2,14 @@ use crate::{ io::{Filesystem, WitxIo}, parser::{ CommentSyntax, DeclSyntax, Documented, EnumSyntax, FlagsSyntax, HandleSyntax, - ImportTypeSyntax, IntSyntax, ModuleDeclSyntax, StructSyntax, TypedefSyntax, UnionSyntax, - VariantSyntax, + ImportTypeSyntax, IntSyntax, InterfaceFuncSyntax, ModuleDeclSyntax, ProfileDeclSyntax, + StructSyntax, TypedefSyntax, UnionSyntax, VariantSyntax, }, BuiltinType, Definition, Entry, EnumDatatype, EnumVariant, FlagsDatatype, FlagsMember, HandleDatatype, Id, IntConst, IntDatatype, IntRepr, InterfaceFunc, InterfaceFuncParam, InterfaceFuncParamPosition, Location, Module, ModuleDefinition, ModuleEntry, ModuleImport, - ModuleImportVariant, NamedType, StructDatatype, StructMember, Type, TypePassedBy, TypeRef, - UnionDatatype, UnionVariant, + ModuleImportVariant, NamedType, Profile, ProfileDefinition, ProfileEntry, ProfileImport, + StructDatatype, StructMember, Type, TypePassedBy, TypeRef, UnionDatatype, UnionVariant, }; use std::collections::{hash_map, HashMap}; use std::path::Path; @@ -167,6 +167,10 @@ impl DocValidationScope<'_> { self.doc.scope.get(name.name(), loc) } + fn entry(&self, name: &Id) -> Option<&Entry> { + self.doc.entries.get(name) + } + pub fn validate_decl( &mut self, decl: &DeclSyntax, @@ -208,6 +212,26 @@ impl DocValidationScope<'_> { .insert(name, Entry::Module(Rc::downgrade(&rc_module))); Ok(Definition::Module(rc_module)) } + DeclSyntax::Profile(syntax) => { + let name = self.introduce(&syntax.name)?; + + let mut profile_validator = ProfileValidation::new(self); + let definitions = syntax + .decls + .iter() + .map(|d| profile_validator.validate_decl(&d)) + .collect::, _>>()?; + let rc_profile = Rc::new(Profile::new( + name.clone(), + definitions, + profile_validator.entries, + comments.docs(), + )); + self.doc + .entries + .insert(name, Entry::Profile(Rc::downgrade(&rc_profile))); + Ok(Definition::Profile(rc_profile)) + } } } @@ -546,68 +570,135 @@ impl<'a> ModuleValidation<'a> { Ok(ModuleDefinition::Import(rc_import)) } ModuleDeclSyntax::Func(syntax) => { - let loc = self.doc.location(syntax.export_loc); - let name = self.scope.introduce(syntax.export, loc)?; - let mut argnames = IdentValidation::new(); - let params = syntax - .params - .iter() - .enumerate() - .map(|(ix, f)| { - Ok(InterfaceFuncParam { - name: argnames.introduce( - f.item.name.name(), - self.doc.location(f.item.name.span()), - )?, - tref: self.doc.validate_datatype( - &f.item.type_, - false, - f.item.name.span(), - )?, - position: InterfaceFuncParamPosition::Param(ix), - docs: f.comments.docs(), - }) - }) - .collect::, _>>()?; - let results = syntax - .results - .iter() - .enumerate() - .map(|(ix, f)| { - let tref = - self.doc - .validate_datatype(&f.item.type_, false, f.item.name.span())?; - if ix == 0 { - match tref.type_().passed_by() { - TypePassedBy::Value(_) => {} - _ => Err(ValidationError::InvalidFirstResultType { - location: self.doc.location(f.item.name.span()), - })?, - } - } - Ok(InterfaceFuncParam { - name: argnames.introduce( - f.item.name.name(), - self.doc.location(f.item.name.span()), - )?, - tref, - position: InterfaceFuncParamPosition::Result(ix), - docs: f.comments.docs(), - }) - }) - .collect::, _>>()?; - let noreturn = syntax.noreturn; + let func = Rc::new(func_validation( + syntax, + &decl.comments, + &mut self.scope, + &self.doc, + )?); + self.entries + .insert(func.name.clone(), ModuleEntry::Func(Rc::downgrade(&func))); + Ok(ModuleDefinition::Func(func)) + } + } + } +} - let rc_func = Rc::new(InterfaceFunc { - name: name.clone(), - params, - results, - noreturn, +fn func_validation( + syntax: &InterfaceFuncSyntax, + comments: &CommentSyntax, + scope: &mut IdentValidation, + doc: &DocValidationScope, +) -> Result { + let loc = doc.location(syntax.export_loc); + let name = scope.introduce(syntax.export, loc)?; + let mut argnames = IdentValidation::new(); + let params = syntax + .params + .iter() + .enumerate() + .map(|(ix, f)| { + Ok(InterfaceFuncParam { + name: argnames.introduce(f.item.name.name(), doc.location(f.item.name.span()))?, + tref: doc.validate_datatype(&f.item.type_, false, f.item.name.span())?, + position: InterfaceFuncParamPosition::Param(ix), + docs: f.comments.docs(), + }) + }) + .collect::, _>>()?; + let results = syntax + .results + .iter() + .enumerate() + .map(|(ix, f)| { + let tref = doc.validate_datatype(&f.item.type_, false, f.item.name.span())?; + if ix == 0 { + match tref.type_().passed_by() { + TypePassedBy::Value(_) => {} + _ => Err(ValidationError::InvalidFirstResultType { + location: doc.location(f.item.name.span()), + })?, + } + } + Ok(InterfaceFuncParam { + name: argnames.introduce(f.item.name.name(), doc.location(f.item.name.span()))?, + tref, + position: InterfaceFuncParamPosition::Result(ix), + docs: f.comments.docs(), + }) + }) + .collect::, _>>()?; + let noreturn = syntax.noreturn; + Ok(InterfaceFunc { + name: name.clone(), + params, + results, + noreturn, + docs: comments.docs(), + }) +} + +struct ProfileValidation<'a> { + doc: &'a DocValidationScope<'a>, + scope: IdentValidation, + pub entries: HashMap, +} + +impl<'a> ProfileValidation<'a> { + fn new(doc: &'a DocValidationScope<'a>) -> Self { + Self { + doc, + scope: IdentValidation::new(), + entries: HashMap::new(), + } + } + + fn validate_decl( + &mut self, + decl: &Documented, + ) -> Result { + match &decl.item { + ProfileDeclSyntax::Import(syntax) => { + let name = self.doc.get(syntax)?; + let entry = self + .doc + .entry(&name) + // Only way we have a name that lacks an entry is a recursive definition: + .ok_or_else(|| ValidationError::Recursive { + name: name.as_str().to_string(), + location: self.doc.location(syntax.span()), + })?; + let module = match entry { + Entry::Module(weak_ref) => { + weak_ref.upgrade().expect("weak backref to defined type") + } + _ => Err(ValidationError::WrongKindName { + expected: "module", + got: entry.kind(), + name: name.as_str().to_string(), + location: self.doc.location(syntax.span()), + })?, + }; + let prof_import = Rc::new(ProfileImport { + module, docs: decl.comments.docs(), }); + self.entries.insert( + name.clone(), + ProfileEntry::Import(Rc::downgrade(&prof_import)), + ); + Ok(ProfileDefinition::Import(prof_import)) + } + ProfileDeclSyntax::Func(syntax) => { + let func = Rc::new(func_validation( + syntax, + &decl.comments, + &mut self.scope, + &self.doc, + )?); self.entries - .insert(name, ModuleEntry::Func(Rc::downgrade(&rc_func))); - Ok(ModuleDefinition::Func(rc_func)) + .insert(func.name.clone(), ProfileEntry::Func(Rc::downgrade(&func))); + Ok(ProfileDefinition::Func(func)) } } } diff --git a/tools/witx/tests/wasi.rs b/tools/witx/tests/wasi.rs index 17f79341d..a11868b80 100644 --- a/tools/witx/tests/wasi.rs +++ b/tools/witx/tests/wasi.rs @@ -62,6 +62,10 @@ fn render_roundtrip() { assert_eq!(func, func2); } } + for profile in doc.profiles() { + let profile2 = doc2.profile(&profile.name).expect("doc2 missing profile"); + assert_eq!(profile, profile2); + } } // This should be equivelant to the above, but just in case some code changes where it isnt: assert_eq!(doc, doc2); From 790fe579fbb4cbc264eee5591ac90789e6fa4c7e Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 29 Apr 2020 11:18:40 -0700 Subject: [PATCH 2/8] Add profile to wasi_snapshot_preview1.witx describing an executable --- phases/snapshot/witx/wasi_snapshot_preview1.witx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/phases/snapshot/witx/wasi_snapshot_preview1.witx b/phases/snapshot/witx/wasi_snapshot_preview1.witx index 98cd94787..287e74ebe 100644 --- a/phases/snapshot/witx/wasi_snapshot_preview1.witx +++ b/phases/snapshot/witx/wasi_snapshot_preview1.witx @@ -530,3 +530,13 @@ (result $error $errno) ) ) + +;;; The WASI standard specifies executables as having all of the +;;; WASI modules available for import, and exporting a start function. +(profile $wasi_snapshot_preview1_executable + ;;; Executables must export a start function: + (@interface func (export "_start")) + ;;; Can import from wasi snapshot preview1: + (import $wasi_snapshot_preview1) +) + From 7920907ca1a4f968c710143314729bc180bcfac2 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 29 Apr 2020 11:24:11 -0700 Subject: [PATCH 3/8] ephemeral: add a wasi_ephemeral_profile.witx that specifies an executable --- .../witx/wasi_ephemeral_profile.witx | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 phases/ephemeral/witx/wasi_ephemeral_profile.witx diff --git a/phases/ephemeral/witx/wasi_ephemeral_profile.witx b/phases/ephemeral/witx/wasi_ephemeral_profile.witx new file mode 100644 index 000000000..bde085e1d --- /dev/null +++ b/phases/ephemeral/witx/wasi_ephemeral_profile.witx @@ -0,0 +1,27 @@ +(use "wasi_ephemeral_args.witx") +(use "wasi_ephemeral_clock.witx") +(use "wasi_ephemeral_environ.witx") +(use "wasi_ephemeral_fd.witx") +(use "wasi_ephemeral_path.witx") +(use "wasi_ephemeral_poll.witx") +(use "wasi_ephemeral_proc.witx") +(use "wasi_ephemeral_random.witx") +(use "wasi_ephemeral_sched.witx") +(use "wasi_ephemeral_sock.witx") + +;;; The WASI standard specifies executables as having all of the +;;; WASI modules available for import, and exporting a start function. +(profile $wasi_ephemeral_executable + ;;; Executables must export a start function: + (@interface func (export "_start")) + (import $wasi_ephemeral_args) + (import $wasi_ephemeral_clock) + (import $wasi_ephemeral_environ) + (import $wasi_ephemeral_fd) + (import $wasi_ephemeral_path) + (import $wasi_ephemeral_poll) + (import $wasi_ephemeral_proc) + (import $wasi_ephemeral_random) + (import $wasi_ephemeral_sched) + (import $wasi_ephemeral_sock) +) From aa26197a32ce98730576903fa0a8558eedcf0e17 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 29 Apr 2020 11:24:38 -0700 Subject: [PATCH 4/8] witx tests: use the profile.witx as the umbrella for ephemeral --- tools/witx/src/phases.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/tools/witx/src/phases.rs b/tools/witx/src/phases.rs index 0bf697e21..5eeac71a3 100644 --- a/tools/witx/src/phases.rs +++ b/tools/witx/src/phases.rs @@ -23,16 +23,9 @@ pub fn ephemeral() -> Result> { let root = repo_root()?; let ephemeral = root.join("phases/ephemeral/witx"); let paths = vec![ - ephemeral.join("wasi_ephemeral_args.witx"), - ephemeral.join("wasi_ephemeral_clock.witx"), - ephemeral.join("wasi_ephemeral_environ.witx"), - ephemeral.join("wasi_ephemeral_fd.witx"), - ephemeral.join("wasi_ephemeral_path.witx"), - ephemeral.join("wasi_ephemeral_poll.witx"), - ephemeral.join("wasi_ephemeral_proc.witx"), - ephemeral.join("wasi_ephemeral_random.witx"), - ephemeral.join("wasi_ephemeral_sched.witx"), - ephemeral.join("wasi_ephemeral_sock.witx"), + // Ephemeral is split into many files (one per module) but the profile witx `use`s them + // all: + ephemeral.join("wasi_ephemeral_profile.witx"), ]; ensure_exists(&paths)?; Ok(paths) From f4efd3f804019b573762246b92f4974fc664f3c6 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 29 Apr 2020 15:13:19 -0700 Subject: [PATCH 5/8] s/executable/command, and put the dunce cap right on my head --- ...i_ephemeral_profile.witx => wasi_ephemeral_command.witx} | 6 +++--- phases/snapshot/witx/wasi_snapshot_preview1.witx | 6 +++--- tools/witx/src/phases.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) rename phases/ephemeral/witx/{wasi_ephemeral_profile.witx => wasi_ephemeral_command.witx} (83%) diff --git a/phases/ephemeral/witx/wasi_ephemeral_profile.witx b/phases/ephemeral/witx/wasi_ephemeral_command.witx similarity index 83% rename from phases/ephemeral/witx/wasi_ephemeral_profile.witx rename to phases/ephemeral/witx/wasi_ephemeral_command.witx index bde085e1d..625ddcf8f 100644 --- a/phases/ephemeral/witx/wasi_ephemeral_profile.witx +++ b/phases/ephemeral/witx/wasi_ephemeral_command.witx @@ -9,10 +9,10 @@ (use "wasi_ephemeral_sched.witx") (use "wasi_ephemeral_sock.witx") -;;; The WASI standard specifies executables as having all of the +;;; The WASI standard specifies commands as having all of the ;;; WASI modules available for import, and exporting a start function. -(profile $wasi_ephemeral_executable - ;;; Executables must export a start function: +(profile $wasi_ephemeral_command + ;;; Commands must export a start function: (@interface func (export "_start")) (import $wasi_ephemeral_args) (import $wasi_ephemeral_clock) diff --git a/phases/snapshot/witx/wasi_snapshot_preview1.witx b/phases/snapshot/witx/wasi_snapshot_preview1.witx index 287e74ebe..53ea40a18 100644 --- a/phases/snapshot/witx/wasi_snapshot_preview1.witx +++ b/phases/snapshot/witx/wasi_snapshot_preview1.witx @@ -531,10 +531,10 @@ ) ) -;;; The WASI standard specifies executables as having all of the +;;; The WASI standard specifies commands as having all of the ;;; WASI modules available for import, and exporting a start function. -(profile $wasi_snapshot_preview1_executable - ;;; Executables must export a start function: +(profile $wasi_snapshot_preview1_command + ;;; Commands must export a start function: (@interface func (export "_start")) ;;; Can import from wasi snapshot preview1: (import $wasi_snapshot_preview1) diff --git a/tools/witx/src/phases.rs b/tools/witx/src/phases.rs index 5eeac71a3..12cc68da9 100644 --- a/tools/witx/src/phases.rs +++ b/tools/witx/src/phases.rs @@ -23,9 +23,9 @@ pub fn ephemeral() -> Result> { let root = repo_root()?; let ephemeral = root.join("phases/ephemeral/witx"); let paths = vec![ - // Ephemeral is split into many files (one per module) but the profile witx `use`s them + // Ephemeral is split into many files (one per module) but the command witx `use`s them // all: - ephemeral.join("wasi_ephemeral_profile.witx"), + ephemeral.join("wasi_ephemeral_command.witx"), ]; ensure_exists(&paths)?; Ok(paths) From 77229818a40dae9be3335731d045fcd59e82ab80 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 29 Apr 2020 15:55:37 -0700 Subject: [PATCH 6/8] profiles: interfaces are now `expose`d, funcs are now `require`d --- .../witx/wasi_ephemeral_command.witx | 22 +++++------ .../snapshot/witx/wasi_snapshot_preview1.witx | 4 +- tools/witx/src/ast.rs | 36 ++++++++++-------- tools/witx/src/lib.rs | 6 +-- tools/witx/src/parser.rs | 17 +++++---- tools/witx/src/render.rs | 17 +++++++-- tools/witx/src/validate.rs | 37 +++++++++++-------- tools/witx/tests/wasi.rs | 13 +++++++ 8 files changed, 95 insertions(+), 57 deletions(-) diff --git a/phases/ephemeral/witx/wasi_ephemeral_command.witx b/phases/ephemeral/witx/wasi_ephemeral_command.witx index 625ddcf8f..16371815b 100644 --- a/phases/ephemeral/witx/wasi_ephemeral_command.witx +++ b/phases/ephemeral/witx/wasi_ephemeral_command.witx @@ -13,15 +13,15 @@ ;;; WASI modules available for import, and exporting a start function. (profile $wasi_ephemeral_command ;;; Commands must export a start function: - (@interface func (export "_start")) - (import $wasi_ephemeral_args) - (import $wasi_ephemeral_clock) - (import $wasi_ephemeral_environ) - (import $wasi_ephemeral_fd) - (import $wasi_ephemeral_path) - (import $wasi_ephemeral_poll) - (import $wasi_ephemeral_proc) - (import $wasi_ephemeral_random) - (import $wasi_ephemeral_sched) - (import $wasi_ephemeral_sock) + (require (@interface func (export "_start"))) + (expose $wasi_ephemeral_args) + (expose $wasi_ephemeral_clock) + (expose $wasi_ephemeral_environ) + (expose $wasi_ephemeral_fd) + (expose $wasi_ephemeral_path) + (expose $wasi_ephemeral_poll) + (expose $wasi_ephemeral_proc) + (expose $wasi_ephemeral_random) + (expose $wasi_ephemeral_sched) + (expose $wasi_ephemeral_sock) ) diff --git a/phases/snapshot/witx/wasi_snapshot_preview1.witx b/phases/snapshot/witx/wasi_snapshot_preview1.witx index 53ea40a18..6efa0f2f0 100644 --- a/phases/snapshot/witx/wasi_snapshot_preview1.witx +++ b/phases/snapshot/witx/wasi_snapshot_preview1.witx @@ -535,8 +535,8 @@ ;;; WASI modules available for import, and exporting a start function. (profile $wasi_snapshot_preview1_command ;;; Commands must export a start function: - (@interface func (export "_start")) + (require (@interface func (export "_start"))) ;;; Can import from wasi snapshot preview1: - (import $wasi_snapshot_preview1) + (expose $wasi_snapshot_preview1) ) diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index 99166f87f..93ca35a78 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -453,31 +453,31 @@ impl Profile { docs, } } - pub fn import(&self, name: &Id) -> Option> { + pub fn expose(&self, name: &Id) -> Option> { self.entries.get(name).and_then(|e| match e { - ProfileEntry::Import(weak) => { + ProfileEntry::Expose(weak) => { Some(weak.upgrade().expect("always possible to upgrade entry")) } _ => None, }) } - pub fn imports<'a>(&'a self) -> impl Iterator> + 'a { + pub fn exposes<'a>(&'a self) -> impl Iterator> + 'a { self.definitions.iter().filter_map(|d| match d { - ProfileDefinition::Import(def) => Some(def.clone()), + ProfileDefinition::Expose(def) => Some(def.clone()), _ => None, }) } - pub fn func(&self, name: &Id) -> Option> { + pub fn require(&self, name: &Id) -> Option> { self.entries.get(name).and_then(|e| match e { - ProfileEntry::Func(weak) => { + ProfileEntry::Require(weak) => { Some(weak.upgrade().expect("always possible to upgrade entry")) } _ => None, }) } - pub fn funcs<'a>(&'a self) -> impl Iterator> + 'a { + pub fn requires<'a>(&'a self) -> impl Iterator> + 'a { self.definitions.iter().filter_map(|d| match d { - ProfileDefinition::Func(def) => Some(def.clone()), + ProfileDefinition::Require(def) => Some(def.clone()), _ => None, }) } @@ -499,34 +499,40 @@ impl std::hash::Hash for Profile { } #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct ProfileImport { +pub struct ExposedModule { pub module: Rc, pub docs: String, } +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct RequiredFunc { + pub func: Rc, + pub docs: String, +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum ProfileDefinition { - Import(Rc), - Func(Rc), + Expose(Rc), + Require(Rc), } #[derive(Debug, Clone)] pub enum ProfileEntry { - Import(Weak), - Func(Weak), + Expose(Weak), + Require(Weak), } impl PartialEq for ProfileEntry { fn eq(&self, rhs: &ProfileEntry) -> bool { match (self, rhs) { - (ProfileEntry::Import(i), ProfileEntry::Import(i_rhs)) => { + (ProfileEntry::Expose(i), ProfileEntry::Expose(i_rhs)) => { i.upgrade() .expect("always possible to upgrade profileentry when part of profile") == i_rhs .upgrade() .expect("always possible to upgrade profileentry when part of profile") } - (ProfileEntry::Func(i), ProfileEntry::Func(i_rhs)) => { + (ProfileEntry::Require(i), ProfileEntry::Require(i_rhs)) => { i.upgrade() .expect("always possible to upgrade profileentry when part of profile") == i_rhs diff --git a/tools/witx/src/lib.rs b/tools/witx/src/lib.rs index 928f4afbd..b59db5297 100644 --- a/tools/witx/src/lib.rs +++ b/tools/witx/src/lib.rs @@ -24,11 +24,11 @@ mod toplevel; mod validate; pub use ast::{ - BuiltinType, Definition, Document, Entry, EnumDatatype, EnumVariant, FlagsDatatype, - FlagsMember, HandleDatatype, Id, IntConst, IntDatatype, IntRepr, InterfaceFunc, + BuiltinType, Definition, Document, Entry, EnumDatatype, EnumVariant, ExposedModule, + FlagsDatatype, FlagsMember, HandleDatatype, Id, IntConst, IntDatatype, IntRepr, InterfaceFunc, InterfaceFuncParam, InterfaceFuncParamPosition, Module, ModuleDefinition, ModuleEntry, ModuleImport, ModuleImportVariant, NamedType, Profile, ProfileDefinition, ProfileEntry, - ProfileImport, StructDatatype, StructMember, Type, TypeRef, UnionDatatype, UnionVariant, + RequiredFunc, StructDatatype, StructMember, Type, TypeRef, UnionDatatype, UnionVariant, }; pub use coretypes::{AtomType, CoreFuncType, CoreParamSignifies, CoreParamType, TypePassedBy}; pub use docs::Documentation; diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs index 1ac5f4e46..71c5832e8 100644 --- a/tools/witx/src/parser.rs +++ b/tools/witx/src/parser.rs @@ -47,6 +47,8 @@ mod kw { wast::custom_keyword!(u8); wast::custom_keyword!(usize); wast::custom_keyword!(profile); + wast::custom_keyword!(expose); + wast::custom_keyword!(require); } mod annotation { @@ -706,19 +708,20 @@ impl<'a> Parse<'a> for ProfileSyntax<'a> { #[derive(Debug, Clone, PartialEq, Eq)] pub enum ProfileDeclSyntax<'a> { - Import(wast::Id<'a>), - Func(InterfaceFuncSyntax<'a>), + Expose(wast::Id<'a>), + Require(Documented<'a, InterfaceFuncSyntax<'a>>), } impl<'a> Parse<'a> for ProfileDeclSyntax<'a> { fn parse(parser: Parser<'a>) -> Result { parser.parens(|p| { let mut l = p.lookahead1(); - if l.peek::() { - let _ = p.parse::().expect("just peeked kw"); - Ok(ProfileDeclSyntax::Import(p.parse()?)) - } else if l.peek::() { - Ok(ProfileDeclSyntax::Func(p.parse()?)) + if l.peek::() { + let _ = p.parse::().expect("just peeked kw"); + Ok(ProfileDeclSyntax::Expose(p.parse()?)) + } else if l.peek::() { + let _ = p.parse::().expect("just peeked kw"); + p.parens(|pp| Ok(ProfileDeclSyntax::Require(pp.parse()?))) } else { Err(l.error()) } diff --git a/tools/witx/src/render.rs b/tools/witx/src/render.rs index dea2718de..c4a48d8b5 100644 --- a/tools/witx/src/render.rs +++ b/tools/witx/src/render.rs @@ -344,19 +344,28 @@ impl Profile { pub fn to_sexpr(&self) -> SExpr { let header = vec![SExpr::word("profile"), self.name.to_sexpr()]; let definitions = self - .imports() + .exposes() .map(|i| i.to_sexpr()) - .chain(self.funcs().map(|f| f.to_sexpr())) + .chain(self.requires().map(|f| f.to_sexpr())) .collect::>(); SExpr::docs(&self.docs, SExpr::Vec([header, definitions].concat())) } } -impl ProfileImport { +impl ExposedModule { + pub fn to_sexpr(&self) -> SExpr { + SExpr::docs( + &self.docs, + SExpr::Vec(vec![SExpr::word("expose"), self.module.name.to_sexpr()]), + ) + } +} + +impl RequiredFunc { pub fn to_sexpr(&self) -> SExpr { SExpr::docs( &self.docs, - SExpr::Vec(vec![SExpr::word("import"), self.module.name.to_sexpr()]), + SExpr::Vec(vec![SExpr::word("require"), self.func.to_sexpr()]), ) } } diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs index 35eab4a25..ecd1cd9c6 100644 --- a/tools/witx/src/validate.rs +++ b/tools/witx/src/validate.rs @@ -5,11 +5,12 @@ use crate::{ ImportTypeSyntax, IntSyntax, InterfaceFuncSyntax, ModuleDeclSyntax, ProfileDeclSyntax, StructSyntax, TypedefSyntax, UnionSyntax, VariantSyntax, }, - BuiltinType, Definition, Entry, EnumDatatype, EnumVariant, FlagsDatatype, FlagsMember, - HandleDatatype, Id, IntConst, IntDatatype, IntRepr, InterfaceFunc, InterfaceFuncParam, - InterfaceFuncParamPosition, Location, Module, ModuleDefinition, ModuleEntry, ModuleImport, - ModuleImportVariant, NamedType, Profile, ProfileDefinition, ProfileEntry, ProfileImport, - StructDatatype, StructMember, Type, TypePassedBy, TypeRef, UnionDatatype, UnionVariant, + BuiltinType, Definition, Entry, EnumDatatype, EnumVariant, ExposedModule, FlagsDatatype, + FlagsMember, HandleDatatype, Id, IntConst, IntDatatype, IntRepr, InterfaceFunc, + InterfaceFuncParam, InterfaceFuncParamPosition, Location, Module, ModuleDefinition, + ModuleEntry, ModuleImport, ModuleImportVariant, NamedType, Profile, ProfileDefinition, + ProfileEntry, RequiredFunc, StructDatatype, StructMember, Type, TypePassedBy, TypeRef, + UnionDatatype, UnionVariant, }; use std::collections::{hash_map, HashMap}; use std::path::Path; @@ -658,7 +659,7 @@ impl<'a> ProfileValidation<'a> { decl: &Documented, ) -> Result { match &decl.item { - ProfileDeclSyntax::Import(syntax) => { + ProfileDeclSyntax::Expose(syntax) => { let name = self.doc.get(syntax)?; let entry = self .doc @@ -679,26 +680,32 @@ impl<'a> ProfileValidation<'a> { location: self.doc.location(syntax.span()), })?, }; - let prof_import = Rc::new(ProfileImport { + let exposed_mod = Rc::new(ExposedModule { module, docs: decl.comments.docs(), }); self.entries.insert( name.clone(), - ProfileEntry::Import(Rc::downgrade(&prof_import)), + ProfileEntry::Expose(Rc::downgrade(&exposed_mod)), ); - Ok(ProfileDefinition::Import(prof_import)) + Ok(ProfileDefinition::Expose(exposed_mod)) } - ProfileDeclSyntax::Func(syntax) => { + ProfileDeclSyntax::Require(syntax) => { let func = Rc::new(func_validation( - syntax, - &decl.comments, + &syntax.item, + &syntax.comments, &mut self.scope, &self.doc, )?); - self.entries - .insert(func.name.clone(), ProfileEntry::Func(Rc::downgrade(&func))); - Ok(ProfileDefinition::Func(func)) + let required_func = Rc::new(RequiredFunc { + func, + docs: decl.comments.docs(), + }); + self.entries.insert( + required_func.func.name.clone(), + ProfileEntry::Require(Rc::downgrade(&required_func)), + ); + Ok(ProfileDefinition::Require(required_func)) } } } diff --git a/tools/witx/tests/wasi.rs b/tools/witx/tests/wasi.rs index a11868b80..aba4f130f 100644 --- a/tools/witx/tests/wasi.rs +++ b/tools/witx/tests/wasi.rs @@ -64,6 +64,19 @@ fn render_roundtrip() { } for profile in doc.profiles() { let profile2 = doc2.profile(&profile.name).expect("doc2 missing profile"); + for expose in profile.exposes() { + let expose2 = profile2 + .expose(&expose.module.name) + .expect("profile2 missing expose"); + assert_eq!(expose, expose2); + } + for require in profile.requires() { + let require2 = profile2 + .require(&require.func.name) + .expect("profile2 missing require"); + assert_eq!(require, require2); + } + assert_eq!(profile, profile2); } } From 5ac0e9557f7864519fe3d9797d6c52da389fc71d Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 1 May 2020 16:14:12 -0700 Subject: [PATCH 7/8] parser: interface funcs now have a vec of exports validation narrows it down to one export. also, fix some stuff around where doc comments are permitted/dropped - previously we would have just erased a doc comment on `(noreturn)`. --- tools/witx/src/parser.rs | 68 +++++++++++++++++++++++++++++++------- tools/witx/src/validate.rs | 22 ++++++++++-- 2 files changed, 75 insertions(+), 15 deletions(-) diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs index 71c5832e8..7b6eb15a5 100644 --- a/tools/witx/src/parser.rs +++ b/tools/witx/src/parser.rs @@ -1,6 +1,7 @@ use crate::BuiltinType; use wast::lexer::Comment; use wast::parser::{Parse, Parser, Peek, Result}; +use wast::Error; ///! Parser turns s-expressions into unvalidated syntax constructs. ///! conventions: @@ -125,6 +126,12 @@ pub struct CommentSyntax<'a> { pub comments: Vec<&'a str>, } +impl<'a> CommentSyntax<'a> { + pub fn is_empty(&self) -> bool { + self.comments.is_empty() + } +} + impl<'a> Parse<'a> for CommentSyntax<'a> { fn parse(parser: Parser<'a>) -> Result> { let comments = parser.step(|mut cursor| { @@ -587,30 +594,38 @@ impl Parse<'_> for ImportTypeSyntax { #[derive(Debug, Clone)] pub struct InterfaceFuncSyntax<'a> { - pub export: &'a str, - pub export_loc: wast::Span, + pub exports: Vec>, pub params: Vec>>, pub results: Vec>>, pub noreturn: bool, + pub loc: wast::Span, } impl<'a> Parse<'a> for InterfaceFuncSyntax<'a> { fn parse(parser: Parser<'a>) -> Result { + let loc = parser.cur_span(); parser.parse::()?; parser.parse::()?; - let (export_loc, export) = parser.parens(|p| { - p.parse::()?; - Ok((p.cur_span(), p.parse()?)) - })?; - + let mut exports = Vec::new(); let mut params = Vec::new(); let mut results = Vec::new(); let mut noreturn = false; while !parser.is_empty() { + let loc = parser.cur_span(); let func_field = parser.parse::>()?; match func_field.item { + InterfaceFuncField::Export(export) => { + if func_field.comments.is_empty() { + exports.push(export); + } else { + Err(Error::new( + loc, + "Doc comments on exports are not supported".to_owned(), + ))?; + } + } InterfaceFuncField::Param(item) => { params.push(Documented { comments: func_field.comments, @@ -624,22 +639,44 @@ impl<'a> Parse<'a> for InterfaceFuncSyntax<'a> { }); } InterfaceFuncField::Noreturn => { - noreturn = true; + if func_field.comments.is_empty() { + noreturn = true; + } else { + Err(Error::new( + loc, + "Doc comments noreturn is not supported".to_owned(), + ))?; + } } } } Ok(InterfaceFuncSyntax { - export, - export_loc, + exports, params, results, noreturn, + loc, }) } } +#[derive(Debug, Clone)] +pub struct ExportSyntax<'a> { + pub name: &'a str, + pub loc: wast::Span, +} + +impl<'a> PartialEq for ExportSyntax<'a> { + fn eq(&self, other: &ExportSyntax<'_>) -> bool { + // Don't compare location + self.name == other.name + } +} +impl<'a> Eq for ExportSyntax<'a> {} + enum InterfaceFuncField<'a> { + Export(ExportSyntax<'a>), Param(FieldSyntax<'a>), Result(FieldSyntax<'a>), Noreturn, @@ -660,6 +697,13 @@ impl<'a> Parse<'a> for InterfaceFuncField<'a> { name: parser.parse()?, type_: parser.parse()?, })) + } else if l.peek::() { + parser.parse::()?; + let loc = parser.cur_span(); + Ok(InterfaceFuncField::Export(ExportSyntax { + name: parser.parse()?, + loc, + })) } else if l.peek::() { parser.parse::()?; let mut l = parser.lookahead1(); @@ -678,8 +722,8 @@ impl<'a> Parse<'a> for InterfaceFuncField<'a> { impl PartialEq for InterfaceFuncSyntax<'_> { fn eq(&self, other: &InterfaceFuncSyntax<'_>) -> bool { - // skip the `export_loc` field - self.export == other.export + // Ignore loc + self.exports == other.exports && self.params == other.params && self.results == other.results && self.noreturn == other.noreturn diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs index ecd1cd9c6..5f43aacec 100644 --- a/tools/witx/src/validate.rs +++ b/tools/witx/src/validate.rs @@ -51,6 +51,10 @@ pub enum ValidationError { reason: String, location: Location, }, + #[error("Missing export")] + MissingExport { location: Location }, + #[error("Duplicate exports: only one export permitted")] + DuplicateExports { location: Location }, } impl ValidationError { @@ -63,7 +67,9 @@ impl ValidationError { | InvalidRepr { location, .. } | InvalidFirstResultType { location, .. } | AnonymousStructure { location, .. } - | InvalidUnionField { location, .. } => { + | InvalidUnionField { location, .. } + | MissingExport { location, .. } + | DuplicateExports { location, .. } => { format!("{}\n{}", location.highlight_source_with(witxio), &self) } NameAlreadyExists { @@ -591,8 +597,18 @@ fn func_validation( scope: &mut IdentValidation, doc: &DocValidationScope, ) -> Result { - let loc = doc.location(syntax.export_loc); - let name = scope.introduce(syntax.export, loc)?; + let loc = doc.location(syntax.loc); + let name = if syntax.exports.is_empty() { + Err(ValidationError::MissingExport { + location: loc.clone(), + }) + } else if syntax.exports.len() > 1 { + Err(ValidationError::DuplicateExports { + location: doc.location(syntax.exports[1].loc), + }) + } else { + scope.introduce(syntax.exports[0].name, doc.location(syntax.exports[0].loc)) + }?; let mut argnames = IdentValidation::new(); let params = syntax .params From e17191ed1cf872efd6de065007125fe6d988ef4c Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Fri, 1 May 2020 17:48:23 -0700 Subject: [PATCH 8/8] redo syntax of profile imports to use the same structure as module imports --- .../witx/wasi_ephemeral_command.witx | 5 +- .../snapshot/witx/wasi_snapshot_preview1.witx | 5 +- tools/witx/src/ast.rs | 21 +- tools/witx/src/docs/ast.rs | 1 + tools/witx/src/lib.rs | 2 +- tools/witx/src/parser.rs | 33 +-- tools/witx/src/render.rs | 12 +- tools/witx/src/validate.rs | 206 +++++++++++------- tools/witx/tests/wasi.rs | 10 +- 9 files changed, 168 insertions(+), 127 deletions(-) diff --git a/phases/ephemeral/witx/wasi_ephemeral_command.witx b/phases/ephemeral/witx/wasi_ephemeral_command.witx index 16371815b..84cae4aba 100644 --- a/phases/ephemeral/witx/wasi_ephemeral_command.witx +++ b/phases/ephemeral/witx/wasi_ephemeral_command.witx @@ -12,8 +12,9 @@ ;;; The WASI standard specifies commands as having all of the ;;; WASI modules available for import, and exporting a start function. (profile $wasi_ephemeral_command - ;;; Commands must export a start function: - (require (@interface func (export "_start"))) + ;;; Commands must export a start function, which takes no parameters and + ;;; has no results. + (import "_start" (@interface func)) (expose $wasi_ephemeral_args) (expose $wasi_ephemeral_clock) (expose $wasi_ephemeral_environ) diff --git a/phases/snapshot/witx/wasi_snapshot_preview1.witx b/phases/snapshot/witx/wasi_snapshot_preview1.witx index 6efa0f2f0..2b4ade92c 100644 --- a/phases/snapshot/witx/wasi_snapshot_preview1.witx +++ b/phases/snapshot/witx/wasi_snapshot_preview1.witx @@ -534,8 +534,9 @@ ;;; The WASI standard specifies commands as having all of the ;;; WASI modules available for import, and exporting a start function. (profile $wasi_snapshot_preview1_command - ;;; Commands must export a start function: - (require (@interface func (export "_start"))) + ;;; Commands must export a start function, which has no parameters and no + ;;; results. + (import "_start" (@interface func)) ;;; Can import from wasi snapshot preview1: (expose $wasi_snapshot_preview1) ) diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index 93ca35a78..d48be144a 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -406,6 +406,7 @@ pub struct ModuleImport { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum ModuleImportVariant { Memory, + Func(InterfaceFunc), } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -467,17 +468,17 @@ impl Profile { _ => None, }) } - pub fn require(&self, name: &Id) -> Option> { + pub fn import(&self, name: &Id) -> Option> { self.entries.get(name).and_then(|e| match e { - ProfileEntry::Require(weak) => { + ProfileEntry::Import(weak) => { Some(weak.upgrade().expect("always possible to upgrade entry")) } _ => None, }) } - pub fn requires<'a>(&'a self) -> impl Iterator> + 'a { + pub fn imports<'a>(&'a self) -> impl Iterator> + 'a { self.definitions.iter().filter_map(|d| match d { - ProfileDefinition::Require(def) => Some(def.clone()), + ProfileDefinition::Import(def) => Some(def.clone()), _ => None, }) } @@ -504,22 +505,16 @@ pub struct ExposedModule { pub docs: String, } -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct RequiredFunc { - pub func: Rc, - pub docs: String, -} - #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum ProfileDefinition { Expose(Rc), - Require(Rc), + Import(Rc), } #[derive(Debug, Clone)] pub enum ProfileEntry { Expose(Weak), - Require(Weak), + Import(Weak), } impl PartialEq for ProfileEntry { @@ -532,7 +527,7 @@ impl PartialEq for ProfileEntry { .upgrade() .expect("always possible to upgrade profileentry when part of profile") } - (ProfileEntry::Require(i), ProfileEntry::Require(i_rhs)) => { + (ProfileEntry::Import(i), ProfileEntry::Import(i_rhs)) => { i.upgrade() .expect("always possible to upgrade profileentry when part of profile") == i_rhs diff --git a/tools/witx/src/docs/ast.rs b/tools/witx/src/docs/ast.rs index 10dc9f7c2..a805c34b3 100644 --- a/tools/witx/src/docs/ast.rs +++ b/tools/witx/src/docs/ast.rs @@ -260,6 +260,7 @@ impl ToMarkdown for ModuleImport { ModuleImportVariant::Memory => { node.content_ref_mut::().title = "Memory".to_owned(); } + ModuleImportVariant::Func(_) => unimplemented!(), // FIXME } } } diff --git a/tools/witx/src/lib.rs b/tools/witx/src/lib.rs index b59db5297..3159b572b 100644 --- a/tools/witx/src/lib.rs +++ b/tools/witx/src/lib.rs @@ -28,7 +28,7 @@ pub use ast::{ FlagsDatatype, FlagsMember, HandleDatatype, Id, IntConst, IntDatatype, IntRepr, InterfaceFunc, InterfaceFuncParam, InterfaceFuncParamPosition, Module, ModuleDefinition, ModuleEntry, ModuleImport, ModuleImportVariant, NamedType, Profile, ProfileDefinition, ProfileEntry, - RequiredFunc, StructDatatype, StructMember, Type, TypeRef, UnionDatatype, UnionVariant, + StructDatatype, StructMember, Type, TypeRef, UnionDatatype, UnionVariant, }; pub use coretypes::{AtomType, CoreFuncType, CoreParamSignifies, CoreParamType, TypePassedBy}; pub use docs::Documentation; diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs index 7b6eb15a5..3408f34cd 100644 --- a/tools/witx/src/parser.rs +++ b/tools/witx/src/parser.rs @@ -49,7 +49,6 @@ mod kw { wast::custom_keyword!(usize); wast::custom_keyword!(profile); wast::custom_keyword!(expose); - wast::custom_keyword!(require); } mod annotation { @@ -557,7 +556,7 @@ impl<'a> Parse<'a> for ModuleDeclSyntax<'a> { pub struct ModuleImportSyntax<'a> { pub name: &'a str, pub name_loc: wast::Span, - pub type_: ImportTypeSyntax, + pub variant: ModuleImportVariantSyntax<'a>, } impl<'a> Parse<'a> for ModuleImportSyntax<'a> { @@ -566,7 +565,7 @@ impl<'a> Parse<'a> for ModuleImportSyntax<'a> { Ok(ModuleImportSyntax { name: parser.parse()?, name_loc, - type_: parser.parens(|p| p.parse())?, + variant: parser.parens(|p| p.parse())?, }) } } @@ -574,21 +573,29 @@ impl<'a> Parse<'a> for ModuleImportSyntax<'a> { impl PartialEq for ModuleImportSyntax<'_> { fn eq(&self, other: &ModuleImportSyntax<'_>) -> bool { // skip the `name_loc` field - self.name == other.name && self.type_ == other.type_ + self.name == other.name && self.variant == other.variant } } impl Eq for ModuleImportSyntax<'_> {} #[derive(Debug, Clone, PartialEq, Eq)] -pub enum ImportTypeSyntax { +pub enum ModuleImportVariantSyntax<'a> { Memory, + Func(InterfaceFuncSyntax<'a>), } -impl Parse<'_> for ImportTypeSyntax { - fn parse(parser: Parser<'_>) -> Result { - parser.parse::()?; - Ok(ImportTypeSyntax::Memory) +impl<'a> Parse<'a> for ModuleImportVariantSyntax<'a> { + fn parse(parser: Parser<'a>) -> Result { + let mut l = parser.lookahead1(); + if l.peek::() { + Ok(ModuleImportVariantSyntax::Func(parser.parse()?)) + } else if l.peek::() { + parser.parse::()?; + Ok(ModuleImportVariantSyntax::Memory) + } else { + Err(l.error()) + } } } @@ -753,7 +760,7 @@ impl<'a> Parse<'a> for ProfileSyntax<'a> { #[derive(Debug, Clone, PartialEq, Eq)] pub enum ProfileDeclSyntax<'a> { Expose(wast::Id<'a>), - Require(Documented<'a, InterfaceFuncSyntax<'a>>), + Import(Documented<'a, ModuleImportSyntax<'a>>), } impl<'a> Parse<'a> for ProfileDeclSyntax<'a> { @@ -763,9 +770,9 @@ impl<'a> Parse<'a> for ProfileDeclSyntax<'a> { if l.peek::() { let _ = p.parse::().expect("just peeked kw"); Ok(ProfileDeclSyntax::Expose(p.parse()?)) - } else if l.peek::() { - let _ = p.parse::().expect("just peeked kw"); - p.parens(|pp| Ok(ProfileDeclSyntax::Require(pp.parse()?))) + } else if l.peek::() { + let _ = parser.parse::().expect("just peeked kw"); + Ok(ProfileDeclSyntax::Import(p.parse()?)) } else { Err(l.error()) } diff --git a/tools/witx/src/render.rs b/tools/witx/src/render.rs index c4a48d8b5..969303fc6 100644 --- a/tools/witx/src/render.rs +++ b/tools/witx/src/render.rs @@ -273,6 +273,7 @@ impl ModuleImportVariant { pub fn to_sexpr(&self) -> SExpr { match self { ModuleImportVariant::Memory => SExpr::Vec(vec![SExpr::word("memory")]), + ModuleImportVariant::Func(f) => f.to_sexpr(), } } } @@ -346,7 +347,7 @@ impl Profile { let definitions = self .exposes() .map(|i| i.to_sexpr()) - .chain(self.requires().map(|f| f.to_sexpr())) + .chain(self.imports().map(|f| f.to_sexpr())) .collect::>(); SExpr::docs(&self.docs, SExpr::Vec([header, definitions].concat())) } @@ -360,12 +361,3 @@ impl ExposedModule { ) } } - -impl RequiredFunc { - pub fn to_sexpr(&self) -> SExpr { - SExpr::docs( - &self.docs, - SExpr::Vec(vec![SExpr::word("require"), self.func.to_sexpr()]), - ) - } -} diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs index 5f43aacec..3c5fdd465 100644 --- a/tools/witx/src/validate.rs +++ b/tools/witx/src/validate.rs @@ -1,16 +1,16 @@ use crate::{ io::{Filesystem, WitxIo}, parser::{ - CommentSyntax, DeclSyntax, Documented, EnumSyntax, FlagsSyntax, HandleSyntax, - ImportTypeSyntax, IntSyntax, InterfaceFuncSyntax, ModuleDeclSyntax, ProfileDeclSyntax, + CommentSyntax, DeclSyntax, Documented, EnumSyntax, FlagsSyntax, HandleSyntax, IntSyntax, + InterfaceFuncSyntax, ModuleDeclSyntax, ModuleImportVariantSyntax, ProfileDeclSyntax, StructSyntax, TypedefSyntax, UnionSyntax, VariantSyntax, }, BuiltinType, Definition, Entry, EnumDatatype, EnumVariant, ExposedModule, FlagsDatatype, FlagsMember, HandleDatatype, Id, IntConst, IntDatatype, IntRepr, InterfaceFunc, InterfaceFuncParam, InterfaceFuncParamPosition, Location, Module, ModuleDefinition, ModuleEntry, ModuleImport, ModuleImportVariant, NamedType, Profile, ProfileDefinition, - ProfileEntry, RequiredFunc, StructDatatype, StructMember, Type, TypePassedBy, TypeRef, - UnionDatatype, UnionVariant, + ProfileEntry, StructDatatype, StructMember, Type, TypePassedBy, TypeRef, UnionDatatype, + UnionVariant, }; use std::collections::{hash_map, HashMap}; use std::path::Path; @@ -53,8 +53,10 @@ pub enum ValidationError { }, #[error("Missing export")] MissingExport { location: Location }, - #[error("Duplicate exports: only one export permitted")] - DuplicateExports { location: Location }, + #[error("Unpermitted export")] + UnpermittedExport { location: Location }, + #[error("Invalid import: {name} not supported in this context")] + InvalidImport { name: String, location: Location }, } impl ValidationError { @@ -69,7 +71,8 @@ impl ValidationError { | AnonymousStructure { location, .. } | InvalidUnionField { location, .. } | MissingExport { location, .. } - | DuplicateExports { location, .. } => { + | UnpermittedExport { location, .. } + | InvalidImport { location, .. } => { format!("{}\n{}", location.highlight_source_with(witxio), &self) } NameAlreadyExists { @@ -563,9 +566,15 @@ impl<'a> ModuleValidation<'a> { match &decl.item { ModuleDeclSyntax::Import(syntax) => { let loc = self.doc.location(syntax.name_loc); - let name = self.scope.introduce(syntax.name, loc)?; - let variant = match syntax.type_ { - ImportTypeSyntax::Memory => ModuleImportVariant::Memory, + let name = self.scope.introduce(syntax.name, loc.clone())?; + let variant = match syntax.variant { + ModuleImportVariantSyntax::Memory => ModuleImportVariant::Memory, + ModuleImportVariantSyntax::Func { .. } => { + Err(ValidationError::InvalidImport { + name: "func".to_owned(), + location: loc.clone(), + })? + } }; let rc_import = Rc::new(ModuleImport { name: name.clone(), @@ -577,7 +586,7 @@ impl<'a> ModuleValidation<'a> { Ok(ModuleDefinition::Import(rc_import)) } ModuleDeclSyntax::Func(syntax) => { - let func = Rc::new(func_validation( + let func = Rc::new(FuncContext::Export.validate( syntax, &decl.comments, &mut self.scope, @@ -591,68 +600,91 @@ impl<'a> ModuleValidation<'a> { } } -fn func_validation( - syntax: &InterfaceFuncSyntax, - comments: &CommentSyntax, - scope: &mut IdentValidation, - doc: &DocValidationScope, -) -> Result { - let loc = doc.location(syntax.loc); - let name = if syntax.exports.is_empty() { - Err(ValidationError::MissingExport { - location: loc.clone(), - }) - } else if syntax.exports.len() > 1 { - Err(ValidationError::DuplicateExports { - location: doc.location(syntax.exports[1].loc), - }) - } else { - scope.introduce(syntax.exports[0].name, doc.location(syntax.exports[0].loc)) - }?; - let mut argnames = IdentValidation::new(); - let params = syntax - .params - .iter() - .enumerate() - .map(|(ix, f)| { - Ok(InterfaceFuncParam { - name: argnames.introduce(f.item.name.name(), doc.location(f.item.name.span()))?, - tref: doc.validate_datatype(&f.item.type_, false, f.item.name.span())?, - position: InterfaceFuncParamPosition::Param(ix), - docs: f.comments.docs(), - }) - }) - .collect::, _>>()?; - let results = syntax - .results - .iter() - .enumerate() - .map(|(ix, f)| { - let tref = doc.validate_datatype(&f.item.type_, false, f.item.name.span())?; - if ix == 0 { - match tref.type_().passed_by() { - TypePassedBy::Value(_) => {} - _ => Err(ValidationError::InvalidFirstResultType { - location: doc.location(f.item.name.span()), - })?, +enum FuncContext { + Export, + Import(Id), +} + +impl FuncContext { + fn validate( + self, + syntax: &InterfaceFuncSyntax, + comments: &CommentSyntax, + scope: &mut IdentValidation, + doc: &DocValidationScope, + ) -> Result { + let loc = doc.location(syntax.loc); + let name = match self { + FuncContext::Export => { + if syntax.exports.is_empty() { + Err(ValidationError::MissingExport { + location: loc.clone(), + }) + } else if syntax.exports.len() > 1 { + Err(ValidationError::UnpermittedExport { + location: doc.location(syntax.exports[1].loc), + }) + } else { + scope.introduce(syntax.exports[0].name, doc.location(syntax.exports[0].loc)) + } + } + FuncContext::Import(name) => { + if syntax.exports.is_empty() { + Ok(name) + } else { + Err(ValidationError::UnpermittedExport { + location: doc.location(syntax.exports[0].loc), + }) } } - Ok(InterfaceFuncParam { - name: argnames.introduce(f.item.name.name(), doc.location(f.item.name.span()))?, - tref, - position: InterfaceFuncParamPosition::Result(ix), - docs: f.comments.docs(), + }?; + let mut argnames = IdentValidation::new(); + let params = syntax + .params + .iter() + .enumerate() + .map(|(ix, f)| { + Ok(InterfaceFuncParam { + name: argnames + .introduce(f.item.name.name(), doc.location(f.item.name.span()))?, + tref: doc.validate_datatype(&f.item.type_, false, f.item.name.span())?, + position: InterfaceFuncParamPosition::Param(ix), + docs: f.comments.docs(), + }) }) + .collect::, _>>()?; + let results = syntax + .results + .iter() + .enumerate() + .map(|(ix, f)| { + let tref = doc.validate_datatype(&f.item.type_, false, f.item.name.span())?; + if ix == 0 { + match tref.type_().passed_by() { + TypePassedBy::Value(_) => {} + _ => Err(ValidationError::InvalidFirstResultType { + location: doc.location(f.item.name.span()), + })?, + } + } + Ok(InterfaceFuncParam { + name: argnames + .introduce(f.item.name.name(), doc.location(f.item.name.span()))?, + tref, + position: InterfaceFuncParamPosition::Result(ix), + docs: f.comments.docs(), + }) + }) + .collect::, _>>()?; + let noreturn = syntax.noreturn; + Ok(InterfaceFunc { + name: name.clone(), + params, + results, + noreturn, + docs: comments.docs(), }) - .collect::, _>>()?; - let noreturn = syntax.noreturn; - Ok(InterfaceFunc { - name: name.clone(), - params, - results, - noreturn, - docs: comments.docs(), - }) + } } struct ProfileValidation<'a> { @@ -706,22 +738,34 @@ impl<'a> ProfileValidation<'a> { ); Ok(ProfileDefinition::Expose(exposed_mod)) } - ProfileDeclSyntax::Require(syntax) => { - let func = Rc::new(func_validation( - &syntax.item, - &syntax.comments, - &mut self.scope, - &self.doc, - )?); - let required_func = Rc::new(RequiredFunc { - func, + ProfileDeclSyntax::Import(syntax) => { + let loc = self.doc.location(syntax.item.name_loc); + let name = self.scope.introduce(syntax.item.name, loc.clone())?; + let variant = match &syntax.item.variant { + ModuleImportVariantSyntax::Memory => Err(ValidationError::InvalidImport { + name: "memory".to_owned(), + location: loc.clone(), + })?, + ModuleImportVariantSyntax::Func(f) => { + ModuleImportVariant::Func(FuncContext::Import(name.clone()).validate( + &f, + &syntax.comments, + &mut self.scope, + &self.doc, + )?) + } + }; + let rc_import = Rc::new(ModuleImport { + name: name.clone(), + variant, docs: decl.comments.docs(), }); + self.entries.insert( - required_func.func.name.clone(), - ProfileEntry::Require(Rc::downgrade(&required_func)), + rc_import.name.clone(), + ProfileEntry::Import(Rc::downgrade(&rc_import)), ); - Ok(ProfileDefinition::Require(required_func)) + Ok(ProfileDefinition::Import(rc_import)) } } } diff --git a/tools/witx/tests/wasi.rs b/tools/witx/tests/wasi.rs index aba4f130f..ff9075d36 100644 --- a/tools/witx/tests/wasi.rs +++ b/tools/witx/tests/wasi.rs @@ -70,11 +70,11 @@ fn render_roundtrip() { .expect("profile2 missing expose"); assert_eq!(expose, expose2); } - for require in profile.requires() { - let require2 = profile2 - .require(&require.func.name) - .expect("profile2 missing require"); - assert_eq!(require, require2); + for import in profile.imports() { + let import2 = profile2 + .import(&import.name) + .expect("profile2 missing import"); + assert_eq!(import, import2); } assert_eq!(profile, profile2);