From 8e99732a8bace39bca254c49da1a2cfac391cdda Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 27 Nov 2019 17:44:07 -0800 Subject: [PATCH 01/15] wip: some attempt at reasoning about what interfaces can be polyfilled --- tools/witx/src/ast.rs | 17 +++- tools/witx/src/lib.rs | 2 + tools/witx/src/main.rs | 189 ++++++++++++++++++++++++++++++++++----- tools/witx/src/parser.rs | 15 +--- 4 files changed, 186 insertions(+), 37 deletions(-) diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index 2295cc94c..49fd56b1d 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -2,8 +2,6 @@ use std::collections::HashMap; use std::rc::{Rc, Weak}; -pub use crate::parser::BuiltinType; - #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Id(String); @@ -165,6 +163,21 @@ impl Type { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BuiltinType { + String, + U8, + U16, + U32, + U64, + S8, + S16, + S32, + S64, + F32, + F64, +} + #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum IntRepr { U8, diff --git a/tools/witx/src/lib.rs b/tools/witx/src/lib.rs index ef32bf882..560bdcb71 100644 --- a/tools/witx/src/lib.rs +++ b/tools/witx/src/lib.rs @@ -14,6 +14,7 @@ mod render; mod toplevel; /// Validate declarations into ast mod validate; +mod representation; pub use ast::{ BuiltinType, Definition, Document, Entry, EnumDatatype, EnumVariant, FlagsDatatype, @@ -28,6 +29,7 @@ pub use io::{Filesystem, MockFs, WitxIo}; pub use parser::DeclSyntax; pub use render::SExpr; pub use validate::ValidationError; +pub use representation::Representable; use std::path::{Path, PathBuf}; use thiserror::Error; diff --git a/tools/witx/src/main.rs b/tools/witx/src/main.rs index 6251c3efa..359f2df1d 100644 --- a/tools/witx/src/main.rs +++ b/tools/witx/src/main.rs @@ -1,7 +1,7 @@ use clap::{App, Arg, SubCommand}; +use std::collections::HashMap; use std::fs::File; use std::io::Write; -use std::path::PathBuf; use std::process; use witx::{load, Documentation}; @@ -33,37 +33,184 @@ pub fn main() { .required(false), ), ) + .subcommand( + SubCommand::with_name("polyfill") + .about("Examine differences between interfaces") + .arg( + Arg::with_name("older_interface") + .required(true) + .multiple(true) + .help("path to root of witx document describing interface to polyfill"), + ) + .arg( + Arg::with_name("module_mapping") + .short("m") + .long("module_mapping") + .required(false) + .takes_value(true) + .multiple(true) + .help("module to examine. Use newname=oldname syntax if name is different between new and old interfaces"), + ), + ) .get_matches(); let inputs = app .values_of("input") .expect("at least one input required") - .into_iter() - .map(|i| PathBuf::from(i)) - .collect::>(); + .collect::>(); - match load(&inputs) { - Ok(doc) => { - if app.is_present("verbose") { - println!("{:?}", doc) + let load_docs = { + |inputs: &[&str]| match load(inputs) { + Ok(doc) => doc, + Err(e) => { + println!("{}", e.report()); + if app.is_present("verbose") { + println!("{:?}", e); + } + process::exit(1) } + } + }; + + let doc = load_docs(&inputs); + if app.is_present("verbose") { + println!("{:?}", doc) + } + + if let Some(docs_command) = app.subcommand_matches("docs") { + let md = doc.to_md(); + if let Some(output) = docs_command.value_of("output") { + let mut file = File::create(output).expect("create output file"); + file.write_all(md.as_bytes()).expect("write output file"); + } else { + println!("{}", md) + } + } else if let Some(polyfill_command) = app.subcommand_matches("polyfill") { + let older_inputs = polyfill_command + .values_of("older_interface") + .expect("at least one older_interface argument required") + .collect::>(); + let older_doc = load_docs(&older_inputs); + + let module_mapping_args = polyfill_command + .values_of("module_mapping") + .expect("at least one module_mapping argument required") + .collect::>(); + let module_mapping = parse_module_mapping(&module_mapping_args); - if let Some(subcommand) = app.subcommand_matches("docs") { - let md = doc.to_md(); - if let Some(output) = subcommand.value_of("output") { - let mut file = File::create(output).expect("create output file"); - file.write_all(md.as_bytes()).expect("write output file"); + polyfill(&doc, &older_doc, &module_mapping); + } +} + +fn parse_module_mapping(ms: &[&str]) -> HashMap { + let mut o = HashMap::new(); + for m in ms { + let s = m.split('=').collect::>(); + if s.len() == 0 { + let mname = s.get(0).unwrap(); + o.insert(mname.to_string(), mname.to_string()); + } else if s.len() == 1 { + let newname = s.get(0).unwrap(); + let oldname = s.get(0).unwrap(); + o.insert(newname.to_string(), oldname.to_string()); + } else { + panic!("invalid module mapping: '{}'", m) + } + } + o +} + +fn polyfill(new: &witx::Document, old: &witx::Document, module_mapping: &HashMap) { + use witx::Representable; + for (newmodulename, oldmodulename) in module_mapping { + let newmodule = new + .module(&witx::Id::new(newmodulename)) + .expect("module exists in new"); + let oldmodule = old + .module(&witx::Id::new(oldmodulename)) + .expect("module exists in old"); + + for oldfunc in oldmodule.funcs() { + if let Some(newfunc) = newmodule.func(&oldfunc.name) { + if newfunc.params.len() != oldfunc.params.len() { + println!( + "{}:{} has different number of params than {}:{}", + newmodulename, + newfunc.name.as_str(), + oldmodulename, + oldfunc.name.as_str() + ) } else { - println!("{}", md) + for (newparam, oldparam) in newfunc.params.iter().zip(oldfunc.params.iter()) { + if newparam.name != oldparam.name { + println!( + "{}:{} param {} doesnt match {}:{} param {}", + newmodulename, + newfunc.name.as_str(), + newparam.name.as_str(), + oldmodulename, + oldfunc.name.as_str(), + oldparam.name.as_str(), + ); + } else if !newparam.tref.representable(&oldparam.tref) { + println!( + "{}:{} param {}:{:?} has incompatible representation with {}:{} param {}:{:?}", + newmodulename, + newfunc.name.as_str(), + newparam.name.as_str(), + newparam.tref, + oldmodulename, + oldfunc.name.as_str(), + oldparam.name.as_str(), + newparam.tref, + ); + } + } } + if newfunc.results.len() != oldfunc.results.len() { + println!( + "{}:{} has different number of results than {}:{}", + newmodulename, + newfunc.name.as_str(), + oldmodulename, + oldfunc.name.as_str() + ) + } else { + for (newresult, oldresult) in newfunc.results.iter().zip(oldfunc.results.iter()) + { + if newresult.name != oldresult.name { + println!( + "{}:{} result {} doesnt match {}:{} result {}", + newmodulename, + newfunc.name.as_str(), + newresult.name.as_str(), + oldmodulename, + oldfunc.name.as_str(), + oldresult.name.as_str(), + ); + } else if !newresult.tref.representable(&oldresult.tref) { + println!( + "{}:{} result {}:{:?} has incompatible representation with {}:{} result {}:{:?}", + newmodulename, + newfunc.name.as_str(), + newresult.name.as_str(), + newresult.tref, + oldmodulename, + oldfunc.name.as_str(), + oldresult.name.as_str(), + newresult.tref, + ); + } + } + } + } else { + println!( + "{}:{} does not correspond to function in {}", + oldmodulename, + oldfunc.name.as_str(), + newmodulename + ); } } - Err(e) => { - println!("{}", e.report()); - if app.is_present("verbose") { - println!("{:?}", e); - } - process::exit(1) - } } } diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs index d8088cd61..8e7e89bb2 100644 --- a/tools/witx/src/parser.rs +++ b/tools/witx/src/parser.rs @@ -1,3 +1,4 @@ +use crate::BuiltinType; use wast::lexer::Comment; use wast::parser::{Cursor, Parse, Parser, Peek, Result}; @@ -41,20 +42,6 @@ mod kw { wast::custom_keyword!(u8); } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum BuiltinType { - String, - U8, - U16, - U32, - U64, - S8, - S16, - S32, - S64, - F32, - F64, -} impl Parse<'_> for BuiltinType { fn parse(parser: Parser<'_>) -> Result { From 819adfec87888c575ffd47702682469f908b11fc Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 27 Nov 2019 17:49:42 -0800 Subject: [PATCH 02/15] forgot to git add --- tools/witx/src/representation.rs | 164 +++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 tools/witx/src/representation.rs diff --git a/tools/witx/src/representation.rs b/tools/witx/src/representation.rs new file mode 100644 index 000000000..95de41fe7 --- /dev/null +++ b/tools/witx/src/representation.rs @@ -0,0 +1,164 @@ +use crate::{ + BuiltinType, EnumDatatype, FlagsDatatype, HandleDatatype, IntRepr, StructDatatype, Type, + TypeRef, UnionDatatype, +}; + +pub trait Representable { + fn representable(&self, by: &Self) -> bool; + // XXX its not enough for this to be a bool. we need to give the recipe with which to represent + // it -does it correspond exactly, does it need an upcast, or does it not correspond at all? + // and so on for each member. + // for structs, one might be able to represent each other, but not in the same memory layout. + // this can be arbitrarily deep due to recursion. for ABI compatibility we may need + // structs to correspond exactly, but maybe builtintypes just need to be representable. for + // polyfilling, we may just need everything to be representable. + // so, really this should return an enum describing what sort of equality we found between + // the two types, and then let the caller make that policy decision. TODO: design exactly that + // enum, i guess? + // also what about equality of typeref? +} + +impl Representable for BuiltinType { + fn representable(&self, by: &Self) -> bool { + // An unsigned integer can be used to represent an unsigned integer of smaller width. + // Otherwise, types must be equal. + match self { + BuiltinType::U8 => match by { + BuiltinType::U64 | BuiltinType::U32 | BuiltinType::U16 | BuiltinType::U8 => true, + _ => false, + }, + BuiltinType::U16 => match by { + BuiltinType::U64 | BuiltinType::U32 | BuiltinType::U16 => true, + _ => false, + }, + BuiltinType::U32 => match by { + BuiltinType::U64 | BuiltinType::U32 => true, + _ => false, + }, + other => by == other, + } + } +} + +impl Representable for IntRepr { + fn representable(&self, by: &Self) -> bool { + // An unsigned integer can be used to represent an unsigned integer of smaller width. + match self { + IntRepr::U8 => true, + IntRepr::U16 => match by { + IntRepr::U16 | IntRepr::U32 | IntRepr::U64 => true, + _ => false, + }, + IntRepr::U32 => match by { + IntRepr::U32 | IntRepr::U64 => true, + _ => false, + }, + IntRepr::U64 => *by == IntRepr::U64, + } + } +} + +impl Representable for EnumDatatype { + fn representable(&self, by: &Self) -> bool { + // Integer representation must be compatible + if !by.repr.representable(&self.repr) { + return false; + } + // For each variant in self, must have variant of same name and position in by: + for (ix, v) in self.variants.iter().enumerate() { + if let Some(by_v) = by.variants.get(ix) { + if by_v.name != v.name { + return false; + } + } else { + return false; + } + } + true + } +} + +impl Representable for FlagsDatatype { + fn representable(&self, by: &Self) -> bool { + // Integer representation must be compatible + if !by.repr.representable(&self.repr) { + return false; + } + // For each flag in self, must have flag of same name and position in by: + for (ix, f) in self.flags.iter().enumerate() { + if let Some(by_f) = by.flags.get(ix) { + if by_f.name != f.name { + return false; + } + } else { + return false; + } + } + true + } +} + +impl Representable for HandleDatatype { + fn representable(&self, by: &Self) -> bool { + // Handles must have the same set of named supertypes. Anonymous supertypes are never + // equal, and the validator should probably make sure these are not allowed, because + // what would that even mean?? + for supertype_ref in self.supertypes.iter() { + match supertype_ref { + TypeRef::Name(nt) => { + if let Some(by_nt) = by.supertypes.iter().find_map(|tref| match tref { + TypeRef::Name(by_nt) if by_nt.name == nt.name => Some(by_nt), + _ => None, + }) { + if !nt.dt.representable(&by_nt.dt) { + return false; + } + } else { + return false; + } + } + TypeRef::Value(_) => { + return false; + } + } + } + true + } +} + +impl Representable for StructDatatype { + fn representable(&self, _by: &Self) -> bool { + unimplemented!( + "this one is hard - need more than a bool for this return type to really describe it" + ) + } +} + +impl Representable for UnionDatatype { + fn representable(&self, _by: &Self) -> bool { + unimplemented!("this one is hard") + } +} + +impl Representable for TypeRef { + fn representable(&self, _by: &Self) -> bool { + unimplemented!("this one is hard - representable by type_() is appropriate in some cases, some times you may want precise name equality as well") + } +} + +impl Representable for Type { + fn representable(&self, by: &Self) -> bool { + match (&self, &by) { + (Type::Enum(s), Type::Enum(b)) => s.representable(b), + (Type::Flags(s), Type::Flags(b)) => s.representable(b), + (Type::Struct(s), Type::Struct(b)) => s.representable(b), + (Type::Union(s), Type::Union(b)) => s.representable(b), + (Type::Handle(s), Type::Handle(b)) => s.representable(b), + (Type::Array(s), Type::Array(b)) => s.representable(b), + (Type::Pointer(s), Type::Pointer(b)) => s.representable(b), + (Type::ConstPointer(s), Type::ConstPointer(b)) => s.representable(b), + (Type::Builtin(s), Type::Builtin(b)) => s.representable(b), + _ => false, + } + } +} From 1eab634083d57f789748b235e43c9ad0104692ed Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 2 Dec 2019 11:59:06 -0800 Subject: [PATCH 03/15] witx polyfill: now outputs something vaguely useful --- tools/witx/src/lib.rs | 2 +- tools/witx/src/main.rs | 26 +++--- tools/witx/src/render.rs | 30 +++---- tools/witx/src/representation.rs | 149 +++++++++++++++++++------------ 4 files changed, 124 insertions(+), 83 deletions(-) diff --git a/tools/witx/src/lib.rs b/tools/witx/src/lib.rs index 560bdcb71..815bccc94 100644 --- a/tools/witx/src/lib.rs +++ b/tools/witx/src/lib.rs @@ -29,7 +29,7 @@ pub use io::{Filesystem, MockFs, WitxIo}; pub use parser::DeclSyntax; pub use render::SExpr; pub use validate::ValidationError; -pub use representation::Representable; +pub use representation::{Representable, RepEquality}; use std::path::{Path, PathBuf}; use thiserror::Error; diff --git a/tools/witx/src/main.rs b/tools/witx/src/main.rs index 359f2df1d..8a62dcf5f 100644 --- a/tools/witx/src/main.rs +++ b/tools/witx/src/main.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use std::fs::File; use std::io::Write; use std::process; -use witx::{load, Documentation}; +use witx::{load, Documentation, RepEquality}; pub fn main() { let app = App::new("witx") @@ -106,12 +106,12 @@ fn parse_module_mapping(ms: &[&str]) -> HashMap { let mut o = HashMap::new(); for m in ms { let s = m.split('=').collect::>(); - if s.len() == 0 { + if s.len() == 1 { let mname = s.get(0).unwrap(); o.insert(mname.to_string(), mname.to_string()); - } else if s.len() == 1 { + } else if s.len() == 2 { let newname = s.get(0).unwrap(); - let oldname = s.get(0).unwrap(); + let oldname = s.get(1).unwrap(); o.insert(newname.to_string(), oldname.to_string()); } else { panic!("invalid module mapping: '{}'", m) @@ -152,17 +152,18 @@ fn polyfill(new: &witx::Document, old: &witx::Document, module_mapping: &HashMap oldfunc.name.as_str(), oldparam.name.as_str(), ); - } else if !newparam.tref.representable(&oldparam.tref) { + } else if newparam.tref.representable(&oldparam.tref) != RepEquality::Eq { println!( - "{}:{} param {}:{:?} has incompatible representation with {}:{} param {}:{:?}", + "{}:{} param {}:{} is {:?} of {}:{} param {}:{}", newmodulename, newfunc.name.as_str(), newparam.name.as_str(), - newparam.tref, + newparam.tref.to_sexpr(), + newparam.tref.representable(&oldparam.tref), oldmodulename, oldfunc.name.as_str(), oldparam.name.as_str(), - newparam.tref, + newparam.tref.to_sexpr(), ); } } @@ -188,17 +189,18 @@ fn polyfill(new: &witx::Document, old: &witx::Document, module_mapping: &HashMap oldfunc.name.as_str(), oldresult.name.as_str(), ); - } else if !newresult.tref.representable(&oldresult.tref) { + } else if newresult.tref.representable(&oldresult.tref) != RepEquality::Eq { println!( - "{}:{} result {}:{:?} has incompatible representation with {}:{} result {}:{:?}", + "{}:{} result {}:{} is {:?} of {}:{} result {}:{}", newmodulename, newfunc.name.as_str(), newresult.name.as_str(), - newresult.tref, + newresult.tref.to_sexpr(), + newresult.tref.representable(&oldresult.tref), oldmodulename, oldfunc.name.as_str(), oldresult.name.as_str(), - newresult.tref, + newresult.tref.to_sexpr(), ); } } diff --git a/tools/witx/src/render.rs b/tools/witx/src/render.rs index c9ec34b1f..880f7613b 100644 --- a/tools/witx/src/render.rs +++ b/tools/witx/src/render.rs @@ -4,7 +4,7 @@ use std::fmt; impl fmt::Display for Document { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { for d in self.typenames() { - write!(f, "{}\n", d.definition_sexpr())?; + write!(f, "{}\n", d.to_sexpr())?; } for m in self.modules() { write!(f, "{}\n", m.to_sexpr())?; @@ -69,13 +69,13 @@ impl SExpr { } impl Id { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { SExpr::ident(self.as_str()) } } impl BuiltinType { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { match self { BuiltinType::String => SExpr::word("string"), BuiltinType::U8 => SExpr::word("u8"), @@ -93,7 +93,7 @@ impl BuiltinType { } impl NamedType { - fn definition_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { let body = self.dt.to_sexpr(); SExpr::docs( &self.docs, @@ -103,7 +103,7 @@ impl NamedType { } impl TypeRef { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { match self { TypeRef::Name(n) => n.name.to_sexpr(), TypeRef::Value(v) => v.to_sexpr(), @@ -112,7 +112,7 @@ impl TypeRef { } impl Type { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { match self { Type::Enum(a) => a.to_sexpr(), Type::Flags(a) => a.to_sexpr(), @@ -136,7 +136,7 @@ impl Type { } impl EnumDatatype { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { let header = vec![SExpr::word("enum"), self.repr.to_sexpr()]; let variants = self .variants @@ -148,7 +148,7 @@ impl EnumDatatype { } impl FlagsDatatype { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { let header = vec![SExpr::word("flags"), self.repr.to_sexpr()]; let flags = self .flags @@ -160,7 +160,7 @@ impl FlagsDatatype { } impl StructDatatype { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { let header = vec![SExpr::word("struct")]; let members = self .members @@ -181,7 +181,7 @@ impl StructDatatype { } impl UnionDatatype { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { let header = vec![SExpr::word("union")]; let variants = self .variants @@ -202,7 +202,7 @@ impl UnionDatatype { } impl HandleDatatype { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { let header = vec![SExpr::word("handle")]; let supertypes = self .supertypes @@ -213,7 +213,7 @@ impl HandleDatatype { } } impl IntRepr { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { match self { IntRepr::U8 => SExpr::word("u8"), IntRepr::U16 => SExpr::word("u16"), @@ -224,7 +224,7 @@ impl IntRepr { } impl Module { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { let header = vec![SExpr::word("module"), self.name.to_sexpr()]; let definitions = self .imports() @@ -236,7 +236,7 @@ impl Module { } impl ModuleImport { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { let variant = match self.variant { ModuleImportVariant::Memory => SExpr::Vec(vec![SExpr::word("memory")]), }; @@ -252,7 +252,7 @@ impl ModuleImport { } impl InterfaceFunc { - fn to_sexpr(&self) -> SExpr { + pub fn to_sexpr(&self) -> SExpr { let header = vec![ SExpr::annot("interface"), SExpr::word("func"), diff --git a/tools/witx/src/representation.rs b/tools/witx/src/representation.rs index 95de41fe7..c91813241 100644 --- a/tools/witx/src/representation.rs +++ b/tools/witx/src/representation.rs @@ -3,103 +3,122 @@ use crate::{ TypeRef, UnionDatatype, }; +// A lattice. Eq + Eq = Eq, SuperSet + any = NotEq, NotEq + any = NotEq. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum RepEquality { + Eq, + Superset, + NotEq, +} + +impl RepEquality { + pub fn join(&self, rhs: &Self) -> Self { + match (self, rhs) { + (RepEquality::Eq, RepEquality::Eq) => RepEquality::Eq, + _ => RepEquality::NotEq, + } + } +} + pub trait Representable { - fn representable(&self, by: &Self) -> bool; - // XXX its not enough for this to be a bool. we need to give the recipe with which to represent - // it -does it correspond exactly, does it need an upcast, or does it not correspond at all? - // and so on for each member. - // for structs, one might be able to represent each other, but not in the same memory layout. - // this can be arbitrarily deep due to recursion. for ABI compatibility we may need - // structs to correspond exactly, but maybe builtintypes just need to be representable. for - // polyfilling, we may just need everything to be representable. - // so, really this should return an enum describing what sort of equality we found between - // the two types, and then let the caller make that policy decision. TODO: design exactly that - // enum, i guess? - // also what about equality of typeref? + fn representable(&self, by: &Self) -> RepEquality; } impl Representable for BuiltinType { - fn representable(&self, by: &Self) -> bool { + fn representable(&self, by: &Self) -> RepEquality { // An unsigned integer can be used to represent an unsigned integer of smaller width. // Otherwise, types must be equal. + if self == by { + return RepEquality::Eq; + } match self { BuiltinType::U8 => match by { - BuiltinType::U64 | BuiltinType::U32 | BuiltinType::U16 | BuiltinType::U8 => true, - _ => false, + BuiltinType::U64 | BuiltinType::U32 | BuiltinType::U16 => RepEquality::Superset, + _ => RepEquality::NotEq, }, BuiltinType::U16 => match by { - BuiltinType::U64 | BuiltinType::U32 | BuiltinType::U16 => true, - _ => false, + BuiltinType::U64 | BuiltinType::U32 => RepEquality::Superset, + _ => RepEquality::NotEq, }, BuiltinType::U32 => match by { - BuiltinType::U64 | BuiltinType::U32 => true, - _ => false, + BuiltinType::U64 => RepEquality::Superset, + _ => RepEquality::NotEq, }, - other => by == other, + _ => RepEquality::NotEq, } } } impl Representable for IntRepr { - fn representable(&self, by: &Self) -> bool { + fn representable(&self, by: &Self) -> RepEquality { + if self == by { + return RepEquality::Eq; + } // An unsigned integer can be used to represent an unsigned integer of smaller width. match self { - IntRepr::U8 => true, IntRepr::U16 => match by { - IntRepr::U16 | IntRepr::U32 | IntRepr::U64 => true, - _ => false, + IntRepr::U32 | IntRepr::U64 => RepEquality::Superset, + _ => RepEquality::NotEq, }, IntRepr::U32 => match by { - IntRepr::U32 | IntRepr::U64 => true, - _ => false, + IntRepr::U64 => RepEquality::Superset, + _ => RepEquality::NotEq, }, - IntRepr::U64 => *by == IntRepr::U64, + _ => RepEquality::NotEq, } } } impl Representable for EnumDatatype { - fn representable(&self, by: &Self) -> bool { + fn representable(&self, by: &Self) -> RepEquality { // Integer representation must be compatible - if !by.repr.representable(&self.repr) { - return false; + if self.repr.representable(&by.repr) == RepEquality::NotEq { + return RepEquality::NotEq; } // For each variant in self, must have variant of same name and position in by: for (ix, v) in self.variants.iter().enumerate() { if let Some(by_v) = by.variants.get(ix) { if by_v.name != v.name { - return false; + return RepEquality::NotEq; } } else { - return false; + return RepEquality::NotEq; } } - true + if by.variants.len() > self.variants.len() { + RepEquality::Superset + } else { + self.repr.representable(&by.repr) + } } } impl Representable for FlagsDatatype { - fn representable(&self, by: &Self) -> bool { + fn representable(&self, by: &Self) -> RepEquality { // Integer representation must be compatible - if !by.repr.representable(&self.repr) { - return false; + if self.repr.representable(&by.repr) == RepEquality::NotEq { + return RepEquality::NotEq; } // For each flag in self, must have flag of same name and position in by: for (ix, f) in self.flags.iter().enumerate() { if let Some(by_f) = by.flags.get(ix) { if by_f.name != f.name { - return false; + return RepEquality::NotEq; } } else { - return false; + return RepEquality::NotEq; } } - true + if by.flags.len() > self.flags.len() { + RepEquality::Superset + } else { + self.repr.representable(&by.repr) + } } } impl Representable for HandleDatatype { - fn representable(&self, by: &Self) -> bool { + fn representable(&self, by: &Self) -> RepEquality { // Handles must have the same set of named supertypes. Anonymous supertypes are never // equal, and the validator should probably make sure these are not allowed, because // what would that even mean?? @@ -110,44 +129,64 @@ impl Representable for HandleDatatype { TypeRef::Name(by_nt) if by_nt.name == nt.name => Some(by_nt), _ => None, }) { - if !nt.dt.representable(&by_nt.dt) { - return false; + if nt.dt.representable(&by_nt.dt) == RepEquality::NotEq { + return RepEquality::NotEq; } } else { - return false; + return RepEquality::NotEq; } } TypeRef::Value(_) => { - return false; + return RepEquality::NotEq; } } } - true + RepEquality::Eq } } impl Representable for StructDatatype { - fn representable(&self, _by: &Self) -> bool { - unimplemented!( - "this one is hard - need more than a bool for this return type to really describe it" - ) + fn representable(&self, by: &Self) -> RepEquality { + if self.members.len() != by.members.len() { + return RepEquality::NotEq; + } + for (m, bym) in self.members.iter().zip(by.members.iter()) { + if m.name != bym.name { + return RepEquality::NotEq; + } + if m.tref.type_().representable(&*bym.tref.type_()) != RepEquality::Eq { + return RepEquality::NotEq; + } + } + RepEquality::Eq } } impl Representable for UnionDatatype { - fn representable(&self, _by: &Self) -> bool { - unimplemented!("this one is hard") + fn representable(&self, by: &Self) -> RepEquality { + if self.variants.len() > by.variants.len() { + return RepEquality::NotEq; + } + for (v, byv) in self.variants.iter().zip(by.variants.iter()) { + if v.name != byv.name { + return RepEquality::NotEq; + } + if v.tref.type_().representable(&*byv.tref.type_()) != RepEquality::Eq { + return RepEquality::NotEq; + } + } + RepEquality::Eq } } impl Representable for TypeRef { - fn representable(&self, _by: &Self) -> bool { - unimplemented!("this one is hard - representable by type_() is appropriate in some cases, some times you may want precise name equality as well") + fn representable(&self, by: &Self) -> RepEquality { + self.type_().representable(&*by.type_()) } } impl Representable for Type { - fn representable(&self, by: &Self) -> bool { + fn representable(&self, by: &Self) -> RepEquality { match (&self, &by) { (Type::Enum(s), Type::Enum(b)) => s.representable(b), (Type::Flags(s), Type::Flags(b)) => s.representable(b), @@ -158,7 +197,7 @@ impl Representable for Type { (Type::Pointer(s), Type::Pointer(b)) => s.representable(b), (Type::ConstPointer(s), Type::ConstPointer(b)) => s.representable(b), (Type::Builtin(s), Type::Builtin(b)) => s.representable(b), - _ => false, + _ => RepEquality::NotEq, } } } From 9499dd39211fe8260c5ee4ee03953677dbfaef30 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 2 Dec 2019 16:40:56 -0800 Subject: [PATCH 04/15] consistiency: NamedType's TypeRef member should also be called tref --- tools/witx/src/ast.rs | 4 ++-- tools/witx/src/docs.rs | 2 +- tools/witx/src/render.rs | 2 +- tools/witx/src/representation.rs | 8 +++++++- tools/witx/src/validate.rs | 4 ++-- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index 49fd56b1d..c3ce4eb64 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -123,13 +123,13 @@ impl TypeRef { #[derive(Debug, Clone, PartialEq, Eq)] pub struct NamedType { pub name: Id, - pub dt: TypeRef, + pub tref: TypeRef, pub docs: String, } impl NamedType { pub fn type_(&self) -> Rc { - self.dt.type_() + self.tref.type_() } } diff --git a/tools/witx/src/docs.rs b/tools/witx/src/docs.rs index 511186a47..a6f79280c 100644 --- a/tools/witx/src/docs.rs +++ b/tools/witx/src/docs.rs @@ -39,7 +39,7 @@ impl BuiltinType { impl Documentation for NamedType { fn to_md(&self) -> String { - let body = match &self.dt { + let body = match &self.tref { TypeRef::Value(v) => match &**v { Type::Enum(a) => a.to_md(), Type::Flags(a) => a.to_md(), diff --git a/tools/witx/src/render.rs b/tools/witx/src/render.rs index 880f7613b..2c08aef40 100644 --- a/tools/witx/src/render.rs +++ b/tools/witx/src/render.rs @@ -94,7 +94,7 @@ impl BuiltinType { impl NamedType { pub fn to_sexpr(&self) -> SExpr { - let body = self.dt.to_sexpr(); + let body = self.tref.to_sexpr(); SExpr::docs( &self.docs, SExpr::Vec(vec![SExpr::word("typename"), self.name.to_sexpr(), body]), diff --git a/tools/witx/src/representation.rs b/tools/witx/src/representation.rs index c91813241..38a599c66 100644 --- a/tools/witx/src/representation.rs +++ b/tools/witx/src/representation.rs @@ -129,7 +129,7 @@ impl Representable for HandleDatatype { TypeRef::Name(by_nt) if by_nt.name == nt.name => Some(by_nt), _ => None, }) { - if nt.dt.representable(&by_nt.dt) == RepEquality::NotEq { + if nt.tref.representable(&by_nt.tref) == RepEquality::NotEq { return RepEquality::NotEq; } } else { @@ -185,6 +185,12 @@ impl Representable for TypeRef { } } +impl Representable for NamedType { + fn representable(&self, by: &Self) -> RepEquality { + self.tref.representable(&by.tref) + } +} + impl Representable for Type { fn representable(&self, by: &Self) -> RepEquality { match (&self, &by) { diff --git a/tools/witx/src/validate.rs b/tools/witx/src/validate.rs index de9e3e49c..a8d832056 100644 --- a/tools/witx/src/validate.rs +++ b/tools/witx/src/validate.rs @@ -164,11 +164,11 @@ impl DocValidationScope<'_> { DeclSyntax::Typename(decl) => { let name = self.introduce(&decl.ident)?; let docs = comments.docs(); - let dt = self.validate_datatype(&decl.def, decl.ident.span())?; + let tref = self.validate_datatype(&decl.def, decl.ident.span())?; let rc_datatype = Rc::new(NamedType { name: name.clone(), - dt, + tref, docs, }); self.doc From b90f6cde0a4610499146ad54fadc40dda432dddb Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Mon, 2 Dec 2019 16:41:14 -0800 Subject: [PATCH 05/15] representation: bugfix, add tests --- tools/witx/src/representation.rs | 92 ++++++++++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 3 deletions(-) diff --git a/tools/witx/src/representation.rs b/tools/witx/src/representation.rs index 38a599c66..819c0e82f 100644 --- a/tools/witx/src/representation.rs +++ b/tools/witx/src/representation.rs @@ -1,6 +1,6 @@ use crate::{ - BuiltinType, EnumDatatype, FlagsDatatype, HandleDatatype, IntRepr, StructDatatype, Type, - TypeRef, UnionDatatype, + BuiltinType, EnumDatatype, FlagsDatatype, HandleDatatype, IntRepr, NamedType, StructDatatype, + Type, TypeRef, UnionDatatype, }; // A lattice. Eq + Eq = Eq, SuperSet + any = NotEq, NotEq + any = NotEq. @@ -175,7 +175,11 @@ impl Representable for UnionDatatype { return RepEquality::NotEq; } } - RepEquality::Eq + if by.variants.len() > self.variants.len() { + RepEquality::Superset + } else { + RepEquality::Eq + } } } @@ -207,3 +211,85 @@ impl Representable for Type { } } } + +#[cfg(test)] +mod test { + use super::*; + use crate::io::MockFs; + use crate::toplevel::parse_witx_with; + use crate::Id; + use std::rc::Rc; + + fn def_type(typename: &str, syntax: &str) -> Rc { + use std::path::Path; + let doc = parse_witx_with(&[Path::new("-")], &MockFs::new(&[("-", syntax)])) + .expect("parse witx doc"); + let t = doc.typename(&Id::new(typename)).expect("defined type"); + // Identity should always be true: + assert_eq!(t.representable(&t), RepEquality::Eq, "identity"); + t + } + + #[test] + fn different_typenames_equal() { + let a = def_type("a", "(typename $a (flags u32 $b $c))"); + let d = def_type("d", "(typename $d (flags u32 $b $c))"); + + assert_eq!(a.representable(&d), RepEquality::Eq); + assert_eq!(d.representable(&a), RepEquality::Eq); + } + + #[test] + fn different_flagnames_not_equal() { + let a = def_type("a", "(typename $a (flags u32 $b $c))"); + let d = def_type("d", "(typename $d (flags u32 $b $e))"); + + assert_eq!(a.representable(&d), RepEquality::NotEq); + assert_eq!(d.representable(&a), RepEquality::NotEq); + } + + #[test] + fn flag_superset() { + let base = def_type("a", "(typename $a (flags u32 $b $c))"); + let extra_flag = def_type("a", "(typename $a (flags u32 $b $c $d))"); + + assert_eq!(base.representable(&extra_flag), RepEquality::Superset); + assert_eq!(extra_flag.representable(&base), RepEquality::NotEq); + + let smaller_size = def_type("a", "(typename $a (flags u16 $b $c))"); + assert_eq!(smaller_size.representable(&base), RepEquality::Superset); + assert_eq!( + smaller_size.representable(&extra_flag), + RepEquality::Superset + ); + assert_eq!(base.representable(&smaller_size), RepEquality::NotEq); + } + + #[test] + fn enum_superset() { + let base = def_type("a", "(typename $a (enum u32 $b $c))"); + let extra_variant = def_type("a", "(typename $a (enum u32 $b $c $d))"); + + assert_eq!(base.representable(&extra_variant), RepEquality::Superset); + assert_eq!(extra_variant.representable(&base), RepEquality::NotEq); + + let smaller_size = def_type("a", "(typename $a (enum u16 $b $c))"); + assert_eq!(smaller_size.representable(&base), RepEquality::Superset); + assert_eq!( + smaller_size.representable(&extra_variant), + RepEquality::Superset + ); + } + + #[test] + fn union_superset() { + let base = def_type("a", "(typename $a (union (field $b u32) (field $c f32)))"); + let extra_variant = def_type( + "a", + "(typename $a (union (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); + } +} From 200594bcfee2dd6c063dfa673fb6503e4d02cb00 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 3 Dec 2019 20:35:02 -0800 Subject: [PATCH 06/15] polyfill: produce a reasonably nice report --- tools/witx/src/docs.rs | 4 +- tools/witx/src/lib.rs | 7 +- tools/witx/src/main.rs | 106 +----------- tools/witx/src/polyfill.rs | 268 +++++++++++++++++++++++++++++++ tools/witx/src/representation.rs | 48 +++--- 5 files changed, 312 insertions(+), 121 deletions(-) create mode 100644 tools/witx/src/polyfill.rs diff --git a/tools/witx/src/docs.rs b/tools/witx/src/docs.rs index a6f79280c..162e36720 100644 --- a/tools/witx/src/docs.rs +++ b/tools/witx/src/docs.rs @@ -20,7 +20,7 @@ impl Documentation for Document { } impl BuiltinType { - fn type_name(&self) -> &'static str { + pub fn type_name(&self) -> &'static str { match self { BuiltinType::String => "string", BuiltinType::U8 => "u8", @@ -58,7 +58,7 @@ impl Documentation for NamedType { } impl TypeRef { - fn type_name(&self) -> String { + pub fn type_name(&self) -> String { match self { TypeRef::Name(n) => n.name.as_str().to_string(), TypeRef::Value(ref v) => match &**v { diff --git a/tools/witx/src/lib.rs b/tools/witx/src/lib.rs index 815bccc94..018b55133 100644 --- a/tools/witx/src/lib.rs +++ b/tools/witx/src/lib.rs @@ -8,13 +8,16 @@ mod docs; mod io; /// Witx syntax parsing from SExprs mod parser; +/// Calculate required polyfill between interfaces +pub mod polyfill; /// Render ast to text mod render; +/// Representational equality of types +mod representation; /// Resolve toplevel `use` declarations across files mod toplevel; /// Validate declarations into ast mod validate; -mod representation; pub use ast::{ BuiltinType, Definition, Document, Entry, EnumDatatype, EnumVariant, FlagsDatatype, @@ -28,8 +31,8 @@ pub use docs::Documentation; pub use io::{Filesystem, MockFs, WitxIo}; pub use parser::DeclSyntax; pub use render::SExpr; +pub use representation::{RepEquality, Representable}; pub use validate::ValidationError; -pub use representation::{Representable, RepEquality}; use std::path::{Path, PathBuf}; use thiserror::Error; diff --git a/tools/witx/src/main.rs b/tools/witx/src/main.rs index 8a62dcf5f..8327ac3fb 100644 --- a/tools/witx/src/main.rs +++ b/tools/witx/src/main.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use std::fs::File; use std::io::Write; use std::process; -use witx::{load, Documentation, RepEquality}; +use witx::{load, Documentation}; pub fn main() { let app = App::new("witx") @@ -98,7 +98,12 @@ pub fn main() { .collect::>(); let module_mapping = parse_module_mapping(&module_mapping_args); - polyfill(&doc, &older_doc, &module_mapping); + let polyfill = witx::polyfill::Polyfill::new(&doc, &older_doc, &module_mapping) + .expect("calculate polyfill"); + println!("{}", polyfill.report()); + if app.is_present("verbose") { + println!("{:?}", polyfill); + } } } @@ -119,100 +124,3 @@ fn parse_module_mapping(ms: &[&str]) -> HashMap { } o } - -fn polyfill(new: &witx::Document, old: &witx::Document, module_mapping: &HashMap) { - use witx::Representable; - for (newmodulename, oldmodulename) in module_mapping { - let newmodule = new - .module(&witx::Id::new(newmodulename)) - .expect("module exists in new"); - let oldmodule = old - .module(&witx::Id::new(oldmodulename)) - .expect("module exists in old"); - - for oldfunc in oldmodule.funcs() { - if let Some(newfunc) = newmodule.func(&oldfunc.name) { - if newfunc.params.len() != oldfunc.params.len() { - println!( - "{}:{} has different number of params than {}:{}", - newmodulename, - newfunc.name.as_str(), - oldmodulename, - oldfunc.name.as_str() - ) - } else { - for (newparam, oldparam) in newfunc.params.iter().zip(oldfunc.params.iter()) { - if newparam.name != oldparam.name { - println!( - "{}:{} param {} doesnt match {}:{} param {}", - newmodulename, - newfunc.name.as_str(), - newparam.name.as_str(), - oldmodulename, - oldfunc.name.as_str(), - oldparam.name.as_str(), - ); - } else if newparam.tref.representable(&oldparam.tref) != RepEquality::Eq { - println!( - "{}:{} param {}:{} is {:?} of {}:{} param {}:{}", - newmodulename, - newfunc.name.as_str(), - newparam.name.as_str(), - newparam.tref.to_sexpr(), - newparam.tref.representable(&oldparam.tref), - oldmodulename, - oldfunc.name.as_str(), - oldparam.name.as_str(), - newparam.tref.to_sexpr(), - ); - } - } - } - if newfunc.results.len() != oldfunc.results.len() { - println!( - "{}:{} has different number of results than {}:{}", - newmodulename, - newfunc.name.as_str(), - oldmodulename, - oldfunc.name.as_str() - ) - } else { - for (newresult, oldresult) in newfunc.results.iter().zip(oldfunc.results.iter()) - { - if newresult.name != oldresult.name { - println!( - "{}:{} result {} doesnt match {}:{} result {}", - newmodulename, - newfunc.name.as_str(), - newresult.name.as_str(), - oldmodulename, - oldfunc.name.as_str(), - oldresult.name.as_str(), - ); - } else if newresult.tref.representable(&oldresult.tref) != RepEquality::Eq { - println!( - "{}:{} result {}:{} is {:?} of {}:{} result {}:{}", - newmodulename, - newfunc.name.as_str(), - newresult.name.as_str(), - newresult.tref.to_sexpr(), - newresult.tref.representable(&oldresult.tref), - oldmodulename, - oldfunc.name.as_str(), - oldresult.name.as_str(), - newresult.tref.to_sexpr(), - ); - } - } - } - } else { - println!( - "{}:{} does not correspond to function in {}", - oldmodulename, - oldfunc.name.as_str(), - newmodulename - ); - } - } - } -} diff --git a/tools/witx/src/polyfill.rs b/tools/witx/src/polyfill.rs new file mode 100644 index 000000000..492ee44fa --- /dev/null +++ b/tools/witx/src/polyfill.rs @@ -0,0 +1,268 @@ +use crate::{Document, Id, InterfaceFunc, InterfaceFuncParam, Module, RepEquality, Representable}; +use std::collections::HashMap; +use std::rc::Rc; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum PolyfillError { + #[error("Module not present: {name:?}")] + ModuleNotPresent { name: Id }, + #[error("Function not present: {name:?}")] + FuncNotPresent { name: Id }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Polyfill { + pub modules: Vec, +} + +impl Polyfill { + pub fn new( + new: &Document, + old: &Document, + module_mapping: &HashMap, // Will need a more sophisticated mapping - what about function names, argument names? + ) -> Result { + let mut modules = Vec::new(); + for (newname, oldname) in module_mapping { + let newname = Id::new(newname); + let oldname = Id::new(oldname); + let newmod = new + .module(&newname) + .ok_or_else(|| PolyfillError::ModuleNotPresent { name: newname })?; + let oldmod = old + .module(&oldname) + .ok_or_else(|| PolyfillError::ModuleNotPresent { name: oldname })?; + modules.push(ModulePolyfill::new(newmod, oldmod)?); + } + Ok(Polyfill { modules }) + } + + pub fn report(&self) -> String { + self.modules + .iter() + .map(|m| m.report()) + .collect::>() + .join("\n") + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ModulePolyfill { + pub new: Rc, + pub old: Rc, + pub funcs: Vec, +} + +impl ModulePolyfill { + pub fn new(new: Rc, old: Rc) -> Result { + let mut funcs = Vec::new(); + for oldfunc in old.funcs() { + let newfunc = new + .func(&oldfunc.name) + .ok_or_else(|| PolyfillError::FuncNotPresent { + name: oldfunc.name.clone(), + })?; + funcs.push(FuncPolyfill::new(newfunc, oldfunc)); + } + Ok(ModulePolyfill { new, old, funcs }) + } + + pub fn report(&self) -> String { + format!( + "Implement module {} in terms of {}:\n\t{}", + self.new.name.as_str(), + self.old.name.as_str(), + self.funcs + .iter() + .map(|f| f.report()) + .collect::>() + .join("\n\t"), + ) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FuncPolyfill { + pub new: Rc, + pub old: Rc, + pub mapped_params: Vec, + pub unknown_params: Vec, + pub mapped_results: Vec, + pub unknown_results: Vec, +} + +impl FuncPolyfill { + pub fn new(new: Rc, old: Rc) -> FuncPolyfill { + let mut mapped_params = Vec::new(); + let mut unknown_params = Vec::new(); + + // Old function is called. Need to map each of its parameters to the new function: + for old_param in old.params.iter() { + if let Some(new_param) = new.params.iter().find(|p| p.name == old_param.name) { + mapped_params.push(ParamPolyfill { + new: new_param.clone(), + old: old_param.clone(), + // Call new param type with old param: + repeq : old_param + .tref + .type_() + .representable(&new_param.tref.type_()), + }) + } else { + unknown_params.push(ParamUnknown::Old(old_param.clone())); + } + } + // Are any new params not covered by the old params? + // This search is O(n^2), but n ought to be small. + for new_param in new.params.iter() { + if mapped_params + .iter() + .find(|m| m.new.name == new_param.name) + .is_none() + { + unknown_params.push(ParamUnknown::New(new_param.clone())); + } + } + + let mut mapped_results = Vec::new(); + let mut unknown_results = Vec::new(); + + // New function has returned. Need to map each of its results to the old function: + for new_result in new.results.iter() { + if let Some(old_result) = old.results.iter().find(|p| p.name == new_result.name) { + mapped_results.push(ParamPolyfill { + new: new_result.clone(), + old: old_result.clone(), + // Return new result type as old result: + repeq : new_result + .tref + .type_() + .representable(&old_result.tref.type_()), + }) + } else { + unknown_results.push(ParamUnknown::New(new_result.clone())); + } + } + + // Are any old results not covered by the new results? + for old_result in old.results.iter() { + if mapped_results + .iter() + .find(|m| m.old.name == old_result.name) + .is_none() + { + unknown_results.push(ParamUnknown::Old(old_result.clone())); + } + } + + FuncPolyfill { + new, + old, + mapped_params, + unknown_params, + mapped_results, + unknown_results, + } + } + + pub fn report(&self) -> String { + if self.full_compat() { + format!("{}: full compatibility", self.new.name.as_str()) + } else { + let name = if self.new.name != self.old.name { + format!("{} => {}", self.old.name.as_str(), self.new.name.as_str()) + } else { + self.new.name.as_str().to_string() + }; + let mut contents = Vec::new(); + for p in self.mapped_params.iter() { + contents.push(if !p.full_compat() { + format!("param {}", p.report()) + } else { + format!("param {}: compatible", p.new.name.as_str()) + }) + } + for u in self.unknown_params.iter() { + contents.push(format!( + "{} param {}: no corresponding result!", + u.which(), + u.param().name.as_str() + )) + } + for r in self.mapped_results.iter() { + contents.push(if !r.full_compat() { + format!("result {}", r.report()) + } else { + format!("result {}: compatible", r.new.name.as_str()) + }) + } + for u in self.unknown_results.iter() { + contents.push(format!( + "{} result {}: no corresponding result!", + u.which(), + u.param().name.as_str() + )) + } + let contents = if contents.is_empty() { + String::new() + } else { + format!(":\n\t\t{}", contents.join("\n\t\t")) + }; + format!("{}{}", name, contents) + } + } + pub fn full_compat(&self) -> bool { + self.new.name == self.old.name + && self.mapped_params.iter().all(|p| p.full_compat()) + && self.unknown_params.is_empty() + && self.mapped_results.iter().all(|p| p.full_compat()) + && self.unknown_results.is_empty() + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ParamPolyfill { + pub new: InterfaceFuncParam, + pub old: InterfaceFuncParam, + pub repeq: RepEquality, +} + +impl ParamPolyfill { + pub fn full_compat(&self) -> bool { + self.new.name == self.old.name && self.repeq == RepEquality::Eq + } + pub fn report(&self) -> String { + let name = if self.new.name != self.old.name { + format!("{} => {}", self.old.name.as_str(), self.new.name.as_str()) + } else { + self.new.name.as_str().to_string() + }; + let repr = match self.repeq { + RepEquality::Eq => "compatible types".to_string(), + RepEquality::Superset => format!("{} is superset-compatible with {}", self.old.tref.type_name(), self.new.tref.type_name()), + RepEquality::NotEq => format!("{} is incompatible with new {}", self.old.tref.type_name(), self.new.tref.type_name()) + }; + format!("{}: {}", name, repr) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ParamUnknown { + Old(InterfaceFuncParam), + New(InterfaceFuncParam), +} + +impl ParamUnknown { + pub fn which(&self) -> &'static str { + match self { + ParamUnknown::Old { .. } => "old", + ParamUnknown::New { .. } => "new", + } + } + pub fn param(&self) -> &InterfaceFuncParam { + match self { + ParamUnknown::Old(p) => &p, + ParamUnknown::New(p) => &p, + } + } +} diff --git a/tools/witx/src/representation.rs b/tools/witx/src/representation.rs index 819c0e82f..3bdd6a792 100644 --- a/tools/witx/src/representation.rs +++ b/tools/witx/src/representation.rs @@ -147,6 +147,10 @@ impl Representable for HandleDatatype { impl Representable for StructDatatype { fn representable(&self, by: &Self) -> RepEquality { + // Structs must have exact structural equality - same members, must + // be Eq, in the same order. + // We would require require a more expressive RepEquality enum to describe which members + // might be supersets. if self.members.len() != by.members.len() { return RepEquality::NotEq; } @@ -164,14 +168,19 @@ impl Representable for StructDatatype { impl Representable for UnionDatatype { fn representable(&self, by: &Self) -> RepEquality { + // Unions must have equal variants, by name (independent of order). If `by` has extra + // variants, its a superset. + // We would require require a more expressive RepEquality enum to describe which variants + // might be supersets. if self.variants.len() > by.variants.len() { return RepEquality::NotEq; } - for (v, byv) in self.variants.iter().zip(by.variants.iter()) { - if v.name != byv.name { - return RepEquality::NotEq; - } - if v.tref.type_().representable(&*byv.tref.type_()) != RepEquality::Eq { + 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 { + return RepEquality::NotEq; + } + } else { return RepEquality::NotEq; } } @@ -231,7 +240,7 @@ mod test { } #[test] - fn different_typenames_equal() { + fn different_typenames() { let a = def_type("a", "(typename $a (flags u32 $b $c))"); let d = def_type("d", "(typename $d (flags u32 $b $c))"); @@ -240,22 +249,17 @@ mod test { } #[test] - fn different_flagnames_not_equal() { - let a = def_type("a", "(typename $a (flags u32 $b $c))"); - let d = def_type("d", "(typename $d (flags u32 $b $e))"); - - assert_eq!(a.representable(&d), RepEquality::NotEq); - assert_eq!(d.representable(&a), RepEquality::NotEq); - } - - #[test] - fn flag_superset() { + fn flags() { let base = def_type("a", "(typename $a (flags u32 $b $c))"); let extra_flag = def_type("a", "(typename $a (flags u32 $b $c $d))"); assert_eq!(base.representable(&extra_flag), RepEquality::Superset); assert_eq!(extra_flag.representable(&base), RepEquality::NotEq); + let different_flagnames = def_type("d", "(typename $d (flags u32 $b $e))"); + assert_eq!(base.representable(&different_flagnames), RepEquality::NotEq); + assert_eq!(different_flagnames.representable(&base), RepEquality::NotEq); + let smaller_size = def_type("a", "(typename $a (flags u16 $b $c))"); assert_eq!(smaller_size.representable(&base), RepEquality::Superset); assert_eq!( @@ -266,7 +270,7 @@ mod test { } #[test] - fn enum_superset() { + fn enum_() { let base = def_type("a", "(typename $a (enum u32 $b $c))"); let extra_variant = def_type("a", "(typename $a (enum u32 $b $c $d))"); @@ -282,7 +286,7 @@ mod test { } #[test] - fn union_superset() { + fn union() { let base = def_type("a", "(typename $a (union (field $b u32) (field $c f32)))"); let extra_variant = def_type( "a", @@ -291,5 +295,13 @@ mod test { 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)))"); + assert_eq!(base.representable(&other_ordering), RepEquality::Eq); + assert_eq!(other_ordering.representable(&base), RepEquality::Eq); + assert_eq!( + other_ordering.representable(&extra_variant), + RepEquality::Superset + ); } } From 541e30ebe607dbabfc584a2845d2394a25f482fc Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 4 Dec 2019 11:03:26 -0800 Subject: [PATCH 07/15] polyfill: move document creation to the right module --- tools/witx/src/docs.rs | 111 +++++++++++++++++++++++++++++++++++++ tools/witx/src/main.rs | 2 +- tools/witx/src/polyfill.rs | 84 +--------------------------- 3 files changed, 114 insertions(+), 83 deletions(-) diff --git a/tools/witx/src/docs.rs b/tools/witx/src/docs.rs index 162e36720..b075f03dc 100644 --- a/tools/witx/src/docs.rs +++ b/tools/witx/src/docs.rs @@ -1,4 +1,6 @@ use crate::ast::*; +use crate::polyfill::*; +use crate::RepEquality; pub trait Documentation { fn to_md(&self) -> String; @@ -236,3 +238,112 @@ impl Documentation for InterfaceFunc { ) } } + +impl Documentation for Polyfill { + fn to_md(&self) -> String { + let module_docs = self + .modules + .iter() + .map(|m| m.to_md()) + .collect::>() + .join("\n"); + let type_docs = format!("TODO"); + format!("# Modules\n{}\n# Types\n{}\n", module_docs, type_docs) + } +} + +impl Documentation for ModulePolyfill { + fn to_md(&self) -> String { + format!( + "## `{}` in terms of `{}`\n{}", + self.new.name.as_str(), + self.old.name.as_str(), + self.funcs + .iter() + .map(|f| f.to_md()) + .collect::>() + .join("\n"), + ) + } +} + +impl Documentation for FuncPolyfill { + fn to_md(&self) -> String { + if self.full_compat() { + format!("* `{}`: full compatibility", self.new.name.as_str()) + } else { + let name = if self.new.name != self.old.name { + format!( + "* `{}` => `{}`", + self.old.name.as_str(), + self.new.name.as_str() + ) + } else { + format!("* `{}`", self.new.name.as_str()) + }; + let mut contents = Vec::new(); + for p in self.mapped_params.iter() { + contents.push(if !p.full_compat() { + format!("param {}", p.to_md()) + } else { + format!("param `{}`: compatible", p.new.name.as_str()) + }) + } + for u in self.unknown_params.iter() { + contents.push(format!( + "{} param `{}`: no corresponding result!", + u.which(), + u.param().name.as_str() + )) + } + for r in self.mapped_results.iter() { + contents.push(if !r.full_compat() { + format!("result {}", r.to_md()) + } else { + format!("result `{}`: compatible", r.new.name.as_str()) + }) + } + for u in self.unknown_results.iter() { + contents.push(format!( + "{} result `{}`: no corresponding result!", + u.which(), + u.param().name.as_str() + )) + } + let contents = if contents.is_empty() { + String::new() + } else { + format!(":\n - {}", contents.join("\n - ")) + }; + format!("{}{}", name, contents) + } + } +} + +impl Documentation for ParamPolyfill { + fn to_md(&self) -> String { + let name = if self.new.name != self.old.name { + format!( + "`{}` => `{}`", + self.old.name.as_str(), + self.new.name.as_str() + ) + } else { + format!("`{}`", self.new.name.as_str()) + }; + let repr = match self.repeq { + RepEquality::Eq => "compatible types".to_string(), + RepEquality::Superset => format!( + "`{}` is superset-compatible with `{}`", + self.old.tref.type_name(), + self.new.tref.type_name() + ), + RepEquality::NotEq => format!( + "`{}` is incompatible with new `{}`", + self.old.tref.type_name(), + self.new.tref.type_name() + ), + }; + format!("{}: {}", name, repr) + } +} diff --git a/tools/witx/src/main.rs b/tools/witx/src/main.rs index 8327ac3fb..d7ff23b55 100644 --- a/tools/witx/src/main.rs +++ b/tools/witx/src/main.rs @@ -100,7 +100,7 @@ pub fn main() { let polyfill = witx::polyfill::Polyfill::new(&doc, &older_doc, &module_mapping) .expect("calculate polyfill"); - println!("{}", polyfill.report()); + println!("{}", polyfill.to_md()); if app.is_present("verbose") { println!("{:?}", polyfill); } diff --git a/tools/witx/src/polyfill.rs b/tools/witx/src/polyfill.rs index 492ee44fa..bb5541072 100644 --- a/tools/witx/src/polyfill.rs +++ b/tools/witx/src/polyfill.rs @@ -36,14 +36,6 @@ impl Polyfill { } Ok(Polyfill { modules }) } - - pub fn report(&self) -> String { - self.modules - .iter() - .map(|m| m.report()) - .collect::>() - .join("\n") - } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -66,19 +58,6 @@ impl ModulePolyfill { } Ok(ModulePolyfill { new, old, funcs }) } - - pub fn report(&self) -> String { - format!( - "Implement module {} in terms of {}:\n\t{}", - self.new.name.as_str(), - self.old.name.as_str(), - self.funcs - .iter() - .map(|f| f.report()) - .collect::>() - .join("\n\t"), - ) - } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -103,7 +82,7 @@ impl FuncPolyfill { new: new_param.clone(), old: old_param.clone(), // Call new param type with old param: - repeq : old_param + repeq: old_param .tref .type_() .representable(&new_param.tref.type_()), @@ -134,7 +113,7 @@ impl FuncPolyfill { new: new_result.clone(), old: old_result.clone(), // Return new result type as old result: - repeq : new_result + repeq: new_result .tref .type_() .representable(&old_result.tref.type_()), @@ -165,52 +144,6 @@ impl FuncPolyfill { } } - pub fn report(&self) -> String { - if self.full_compat() { - format!("{}: full compatibility", self.new.name.as_str()) - } else { - let name = if self.new.name != self.old.name { - format!("{} => {}", self.old.name.as_str(), self.new.name.as_str()) - } else { - self.new.name.as_str().to_string() - }; - let mut contents = Vec::new(); - for p in self.mapped_params.iter() { - contents.push(if !p.full_compat() { - format!("param {}", p.report()) - } else { - format!("param {}: compatible", p.new.name.as_str()) - }) - } - for u in self.unknown_params.iter() { - contents.push(format!( - "{} param {}: no corresponding result!", - u.which(), - u.param().name.as_str() - )) - } - for r in self.mapped_results.iter() { - contents.push(if !r.full_compat() { - format!("result {}", r.report()) - } else { - format!("result {}: compatible", r.new.name.as_str()) - }) - } - for u in self.unknown_results.iter() { - contents.push(format!( - "{} result {}: no corresponding result!", - u.which(), - u.param().name.as_str() - )) - } - let contents = if contents.is_empty() { - String::new() - } else { - format!(":\n\t\t{}", contents.join("\n\t\t")) - }; - format!("{}{}", name, contents) - } - } pub fn full_compat(&self) -> bool { self.new.name == self.old.name && self.mapped_params.iter().all(|p| p.full_compat()) @@ -231,19 +164,6 @@ impl ParamPolyfill { pub fn full_compat(&self) -> bool { self.new.name == self.old.name && self.repeq == RepEquality::Eq } - pub fn report(&self) -> String { - let name = if self.new.name != self.old.name { - format!("{} => {}", self.old.name.as_str(), self.new.name.as_str()) - } else { - self.new.name.as_str().to_string() - }; - let repr = match self.repeq { - RepEquality::Eq => "compatible types".to_string(), - RepEquality::Superset => format!("{} is superset-compatible with {}", self.old.tref.type_name(), self.new.tref.type_name()), - RepEquality::NotEq => format!("{} is incompatible with new {}", self.old.tref.type_name(), self.new.tref.type_name()) - }; - format!("{}: {}", name, repr) - } } #[derive(Debug, Clone, PartialEq, Eq)] From 5397a3f0d9858f6829eda28e79ce51d747aeebfa Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 4 Dec 2019 11:33:53 -0800 Subject: [PATCH 08/15] polyfill: note module when func not found --- tools/witx/src/polyfill.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/witx/src/polyfill.rs b/tools/witx/src/polyfill.rs index bb5541072..729feac3e 100644 --- a/tools/witx/src/polyfill.rs +++ b/tools/witx/src/polyfill.rs @@ -8,7 +8,7 @@ pub enum PolyfillError { #[error("Module not present: {name:?}")] ModuleNotPresent { name: Id }, #[error("Function not present: {name:?}")] - FuncNotPresent { name: Id }, + FuncNotPresent { module: Id, name: Id }, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -52,6 +52,7 @@ impl ModulePolyfill { let newfunc = new .func(&oldfunc.name) .ok_or_else(|| PolyfillError::FuncNotPresent { + module: new.name.clone(), name: oldfunc.name.clone(), })?; funcs.push(FuncPolyfill::new(newfunc, oldfunc)); From 62827803385c01fa6ab5e1647c89b72831b5a116 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 4 Dec 2019 12:09:50 -0800 Subject: [PATCH 09/15] docs: delete todo --- tools/witx/src/docs.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/witx/src/docs.rs b/tools/witx/src/docs.rs index b075f03dc..7f391998f 100644 --- a/tools/witx/src/docs.rs +++ b/tools/witx/src/docs.rs @@ -247,8 +247,7 @@ impl Documentation for Polyfill { .map(|m| m.to_md()) .collect::>() .join("\n"); - let type_docs = format!("TODO"); - format!("# Modules\n{}\n# Types\n{}\n", module_docs, type_docs) + format!("# Modules\n{}\n", module_docs) } } From 1651c1a9c65193f1e6130abe6dd2df3f6a48c9a6 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 4 Dec 2019 12:13:17 -0800 Subject: [PATCH 10/15] rustfmt --- tools/witx/src/parser.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/witx/src/parser.rs b/tools/witx/src/parser.rs index 8e7e89bb2..059a7b6ba 100644 --- a/tools/witx/src/parser.rs +++ b/tools/witx/src/parser.rs @@ -42,7 +42,6 @@ mod kw { wast::custom_keyword!(u8); } - impl Parse<'_> for BuiltinType { fn parse(parser: Parser<'_>) -> Result { let mut l = parser.lookahead1(); From 33d060830ba753963dc66379ab02555ff684c811 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 10 Dec 2019 16:46:36 -0800 Subject: [PATCH 11/15] ast: derive Hash everywhere --- tools/witx/src/ast.rs | 59 +++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index c3ce4eb64..0388673e3 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -62,7 +62,14 @@ impl PartialEq for Document { } impl Eq for Document {} -#[derive(Debug, Clone)] +impl std::hash::Hash for Document { + fn hash(&self, state: &mut H) { + std::hash::Hash::hash(&self.definitions, state); + } +} + + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum Definition { Typename(Rc), Module(Rc), @@ -105,7 +112,7 @@ impl PartialEq for Entry { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum TypeRef { Name(Rc), Value(Rc), @@ -120,7 +127,7 @@ impl TypeRef { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct NamedType { pub name: Id, pub tref: TypeRef, @@ -133,7 +140,7 @@ impl NamedType { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum Type { Enum(EnumDatatype), Flags(FlagsDatatype), @@ -163,7 +170,7 @@ impl Type { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum BuiltinType { String, U8, @@ -178,7 +185,7 @@ pub enum BuiltinType { F64, } -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum IntRepr { U8, U16, @@ -186,55 +193,55 @@ pub enum IntRepr { U64, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct EnumDatatype { pub repr: IntRepr, pub variants: Vec, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct EnumVariant { pub name: Id, pub docs: String, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct FlagsDatatype { pub repr: IntRepr, pub flags: Vec, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct FlagsMember { pub name: Id, pub docs: String, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct StructDatatype { pub members: Vec, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct StructMember { pub name: Id, pub tref: TypeRef, pub docs: String, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct UnionDatatype { pub variants: Vec, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct UnionVariant { pub name: Id, pub tref: TypeRef, pub docs: String, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct HandleDatatype { pub supertypes: Vec, } @@ -291,12 +298,20 @@ impl PartialEq for Module { fn eq(&self, rhs: &Module) -> 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 + self.name == rhs.name && self.entries == rhs.entries && self.docs == rhs.docs } } impl Eq for Module {} -#[derive(Debug, Clone)] +impl std::hash::Hash for Module { + fn hash(&self, state: &mut H) { + std::hash::Hash::hash(&self.name, state); + std::hash::Hash::hash(&self.definitions, state); + std::hash::Hash::hash(&self.docs, state); + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum ModuleDefinition { Import(Rc), Func(Rc), @@ -330,19 +345,19 @@ impl PartialEq for ModuleEntry { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct ModuleImport { pub name: Id, pub variant: ModuleImportVariant, pub docs: String, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum ModuleImportVariant { Memory, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct InterfaceFunc { pub name: Id, pub params: Vec, @@ -350,7 +365,7 @@ pub struct InterfaceFunc { pub docs: String, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct InterfaceFuncParam { pub name: Id, pub tref: TypeRef, @@ -358,7 +373,7 @@ pub struct InterfaceFuncParam { pub docs: String, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum InterfaceFuncParamPosition { Param(usize), Result(usize), From f1a4a420c4ac1caaf955abd271dbb20e952bb9b8 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 10 Dec 2019 16:58:04 -0800 Subject: [PATCH 12/15] polyfills: calculate type polyfills as a set --- tools/witx/src/docs.rs | 2 +- tools/witx/src/polyfill.rs | 105 +++++++++++++++++++++++++++---------- 2 files changed, 79 insertions(+), 28 deletions(-) diff --git a/tools/witx/src/docs.rs b/tools/witx/src/docs.rs index 7f391998f..715e2cb80 100644 --- a/tools/witx/src/docs.rs +++ b/tools/witx/src/docs.rs @@ -330,7 +330,7 @@ impl Documentation for ParamPolyfill { } else { format!("`{}`", self.new.name.as_str()) }; - let repr = match self.repeq { + let repr = match self.repeq() { RepEquality::Eq => "compatible types".to_string(), RepEquality::Superset => format!( "`{}` is superset-compatible with `{}`", diff --git a/tools/witx/src/polyfill.rs b/tools/witx/src/polyfill.rs index 729feac3e..04847508d 100644 --- a/tools/witx/src/polyfill.rs +++ b/tools/witx/src/polyfill.rs @@ -1,5 +1,7 @@ -use crate::{Document, Id, InterfaceFunc, InterfaceFuncParam, Module, RepEquality, Representable}; -use std::collections::HashMap; +use crate::{ + Document, Id, InterfaceFunc, InterfaceFuncParam, Module, RepEquality, Representable, TypeRef, +}; +use std::collections::{HashMap, HashSet}; use std::rc::Rc; use thiserror::Error; @@ -11,7 +13,7 @@ pub enum PolyfillError { FuncNotPresent { module: Id, name: Id }, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Polyfill { pub modules: Vec, } @@ -36,9 +38,17 @@ impl Polyfill { } Ok(Polyfill { modules }) } + + pub fn type_polyfills(&self) -> HashSet { + self.modules + .iter() + .map(|m| m.type_polyfills()) + .flatten() + .collect() + } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct ModulePolyfill { pub new: Rc, pub old: Rc, @@ -59,9 +69,16 @@ impl ModulePolyfill { } Ok(ModulePolyfill { new, old, funcs }) } + pub fn type_polyfills(&self) -> HashSet { + self.funcs + .iter() + .map(|f| f.type_polyfills()) + .flatten() + .collect() + } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct FuncPolyfill { pub new: Rc, pub old: Rc, @@ -79,15 +96,7 @@ impl FuncPolyfill { // Old function is called. Need to map each of its parameters to the new function: for old_param in old.params.iter() { if let Some(new_param) = new.params.iter().find(|p| p.name == old_param.name) { - mapped_params.push(ParamPolyfill { - new: new_param.clone(), - old: old_param.clone(), - // Call new param type with old param: - repeq: old_param - .tref - .type_() - .representable(&new_param.tref.type_()), - }) + mapped_params.push(ParamPolyfill::param(new_param.clone(), old_param.clone())) } else { unknown_params.push(ParamUnknown::Old(old_param.clone())); } @@ -110,15 +119,10 @@ impl FuncPolyfill { // New function has returned. Need to map each of its results to the old function: for new_result in new.results.iter() { if let Some(old_result) = old.results.iter().find(|p| p.name == new_result.name) { - mapped_results.push(ParamPolyfill { - new: new_result.clone(), - old: old_result.clone(), - // Return new result type as old result: - repeq: new_result - .tref - .type_() - .representable(&old_result.tref.type_()), - }) + mapped_results.push(ParamPolyfill::result( + new_result.clone(), + old_result.clone(), + )) } else { unknown_results.push(ParamUnknown::New(new_result.clone())); } @@ -152,22 +156,54 @@ impl FuncPolyfill { && self.mapped_results.iter().all(|p| p.full_compat()) && self.unknown_results.is_empty() } + + pub fn type_polyfills(&self) -> HashSet { + self.mapped_params + .iter() + .map(|p| p.type_polyfill.clone()) + .chain(self.mapped_results.iter().map(|p| p.type_polyfill.clone())) + .collect() + } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct ParamPolyfill { pub new: InterfaceFuncParam, pub old: InterfaceFuncParam, - pub repeq: RepEquality, + pub type_polyfill: TypePolyfill, } impl ParamPolyfill { + pub fn param(new: InterfaceFuncParam, old: InterfaceFuncParam) -> Self { + // Call new param type with old param: + let type_polyfill = TypePolyfill::OldToNew(old.tref.clone(), new.tref.clone()); + ParamPolyfill { + new, + old, + type_polyfill, + } + } + + pub fn result(new: InterfaceFuncParam, old: InterfaceFuncParam) -> Self { + // Return old result type from new result: + let type_polyfill = TypePolyfill::NewToOld(new.tref.clone(), old.tref.clone()); + ParamPolyfill { + new, + old, + type_polyfill, + } + } + pub fn full_compat(&self) -> bool { - self.new.name == self.old.name && self.repeq == RepEquality::Eq + self.new.name == self.old.name && self.repeq() == RepEquality::Eq + } + + pub fn repeq(&self) -> RepEquality { + self.type_polyfill.repeq() } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum ParamUnknown { Old(InterfaceFuncParam), New(InterfaceFuncParam), @@ -187,3 +223,18 @@ impl ParamUnknown { } } } + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum TypePolyfill { + NewToOld(TypeRef, TypeRef), + OldToNew(TypeRef, TypeRef), +} + +impl TypePolyfill { + pub fn repeq(&self) -> RepEquality { + match self { + TypePolyfill::NewToOld(new, old) => old.type_().representable(&new.type_()), + TypePolyfill::OldToNew(old, new) => new.type_().representable(&old.type_()), + } + } +} From 4051b2eac9155dd33b8c17091fd5dc6baee55ca1 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Wed, 11 Dec 2019 09:41:23 -0800 Subject: [PATCH 13/15] polyfill: a type polyfill can use a common denominator --- tools/witx/src/docs.rs | 43 +++++++++++++++++++++++++++++++++++++- tools/witx/src/polyfill.rs | 21 ++++++++++++++++--- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/tools/witx/src/docs.rs b/tools/witx/src/docs.rs index 715e2cb80..cbae46bfb 100644 --- a/tools/witx/src/docs.rs +++ b/tools/witx/src/docs.rs @@ -247,7 +247,22 @@ impl Documentation for Polyfill { .map(|m| m.to_md()) .collect::>() .join("\n"); - format!("# Modules\n{}\n", module_docs) + let type_docs = self + .type_polyfills() + .iter() + .filter_map(|t| { + if t.repeq() == RepEquality::Eq { + None + } else { + Some(t.to_md()) + } + }) + .collect::>() + .join("\n"); + format!( + "# Modules\n{}\n# Type Conversions\n{}\n", + module_docs, type_docs + ) } } @@ -346,3 +361,29 @@ impl Documentation for ParamPolyfill { format!("{}: {}", name, repr) } } + +impl Documentation for TypePolyfill { + fn to_md(&self) -> String { + fn repeq_name(r: RepEquality) -> &'static str { + match r { + RepEquality::Eq => ": compatible", + RepEquality::Superset => ": superset", + RepEquality::NotEq => "", + } + } + match self { + TypePolyfill::OldToNew(o, n) => format!( + "* old `{}` => new `{}`{}", + o.type_name(), + n.type_name(), + repeq_name(self.repeq()) + ), + TypePolyfill::NewToOld(n, o) => format!( + "* new `{}` => old `{}`{}", + n.type_name(), + o.type_name(), + repeq_name(self.repeq()) + ), + } + } +} diff --git a/tools/witx/src/polyfill.rs b/tools/witx/src/polyfill.rs index 04847508d..6181d357c 100644 --- a/tools/witx/src/polyfill.rs +++ b/tools/witx/src/polyfill.rs @@ -1,5 +1,6 @@ use crate::{ - Document, Id, InterfaceFunc, InterfaceFuncParam, Module, RepEquality, Representable, TypeRef, + Document, Id, InterfaceFunc, InterfaceFuncParam, Module, RepEquality, Representable, Type, + TypeRef, }; use std::collections::{HashMap, HashSet}; use std::rc::Rc; @@ -174,9 +175,22 @@ pub struct ParamPolyfill { } impl ParamPolyfill { + fn common_denominator(a: TypeRef, b: TypeRef) -> (TypeRef, TypeRef) { + match (&a, &b) { + (TypeRef::Value(va), TypeRef::Value(vb)) => match (&**va, &**vb) { + (Type::Array(a), Type::Array(b)) => (a.clone(), b.clone()), + (Type::Pointer(a), Type::Pointer(b)) => (a.clone(), b.clone()), + (Type::ConstPointer(a), Type::ConstPointer(b)) => (a.clone(), b.clone()), + _ => (a, b), + }, + _ => (a, b), + } + } + pub fn param(new: InterfaceFuncParam, old: InterfaceFuncParam) -> Self { + let (told, tnew) = Self::common_denominator(old.tref.clone(), new.tref.clone()); // Call new param type with old param: - let type_polyfill = TypePolyfill::OldToNew(old.tref.clone(), new.tref.clone()); + let type_polyfill = TypePolyfill::OldToNew(told, tnew); ParamPolyfill { new, old, @@ -185,8 +199,9 @@ impl ParamPolyfill { } pub fn result(new: InterfaceFuncParam, old: InterfaceFuncParam) -> Self { + let (told, tnew) = Self::common_denominator(old.tref.clone(), new.tref.clone()); // Return old result type from new result: - let type_polyfill = TypePolyfill::NewToOld(new.tref.clone(), old.tref.clone()); + let type_polyfill = TypePolyfill::NewToOld(tnew, told); ParamPolyfill { new, old, From 441716f0b62914e05b247effcc9ccc0db7c06ecf Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 17 Dec 2019 16:41:51 -0800 Subject: [PATCH 14/15] add module for calculating C ABI layout of datatypes --- tools/witx/src/layout.rs | 146 +++++++++++++++++++++++++++++++++++++++ tools/witx/src/lib.rs | 2 + 2 files changed, 148 insertions(+) create mode 100644 tools/witx/src/layout.rs diff --git a/tools/witx/src/layout.rs b/tools/witx/src/layout.rs new file mode 100644 index 000000000..626ac67b2 --- /dev/null +++ b/tools/witx/src/layout.rs @@ -0,0 +1,146 @@ +use crate::ast::*; +use std::collections::HashMap; + +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct SizeAlign { + size: usize, + align: usize, +} + +pub trait Layout { + fn mem_size_align(&self) -> SizeAlign; + fn mem_size(&self) -> usize { + self.mem_size_align().size + } + fn mem_align(&self) -> usize { + self.mem_size_align().align + } +} + +impl TypeRef { + fn layout(&self, cache: &mut HashMap) -> SizeAlign { + if let Some(hit) = cache.get(self) { + return *hit; + } + let layout = match &*self.type_() { + Type::Enum(e) => e.repr.layout(), + Type::Flags(f) => f.repr.layout(), + Type::Struct(s) => s.layout(cache), + Type::Union(u) => u.layout(cache), + Type::Handle { .. } => BuiltinType::U32.layout(), + Type::Array { .. } => BuiltinType::String.layout(), + Type::Pointer { .. } | Type::ConstPointer { .. } => BuiltinType::U32.layout(), + Type::Builtin(b) => b.layout(), + }; + 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 IntRepr { + pub fn layout(&self) -> SizeAlign { + match self { + IntRepr::U8 => BuiltinType::U8.layout(), + IntRepr::U16 => BuiltinType::U16.layout(), + IntRepr::U32 => BuiltinType::U32.layout(), + IntRepr::U64 => BuiltinType::U64.layout(), + } + } +} + +pub struct StructMemberLayout<'a> { + member: &'a StructMember, + offset: usize, +} + +impl StructDatatype { + pub fn member_layout( + &self, + cache: &mut HashMap, + ) -> Vec { + let mut members = Vec::new(); + let mut offset = 0; + for m in self.members.iter() { + let sa = m.tref.layout(cache); + offset = align_to(offset, sa.align); + members.push(StructMemberLayout { member: m, offset }); + offset += sa.size; + } + members + } + + pub fn layout(&self, cache: &mut HashMap) -> SizeAlign { + let members = self.member_layout(cache); + let align = members + .iter() + .map(|m| m.member.tref.layout(cache).align) + .max() + .expect("nonzero struct members"); + let last_offset = members.last().expect("nonzero struct members").offset; + let size = align_to(last_offset, align); + SizeAlign { size, align } + } +} + +/// If the next free byte in the struct is `offs`, and the next +/// element has alignment `alignment`, determine the offset at +/// which to place that element. +fn align_to(offs: usize, alignment: usize) -> usize { + offs + alignment - 1 - ((offs + alignment - 1) % alignment) +} + +#[cfg(test)] +mod test { + use super::align_to; + #[test] + fn align() { + assert_eq!(0, align_to(0, 1)); + assert_eq!(0, align_to(0, 2)); + assert_eq!(0, align_to(0, 4)); + assert_eq!(0, align_to(0, 8)); + + assert_eq!(1, align_to(1, 1)); + assert_eq!(2, align_to(1, 2)); + assert_eq!(4, align_to(1, 4)); + assert_eq!(8, align_to(1, 8)); + + assert_eq!(2, align_to(2, 1)); + assert_eq!(2, align_to(2, 2)); + assert_eq!(4, align_to(2, 4)); + assert_eq!(8, align_to(2, 8)); + + assert_eq!(5, align_to(5, 1)); + assert_eq!(6, align_to(5, 2)); + assert_eq!(8, align_to(5, 4)); + assert_eq!(8, align_to(5, 8)); + } +} + +impl UnionDatatype { + pub fn layout(&self, cache: &mut HashMap) -> SizeAlign { + unimplemented!() + } +} + +impl BuiltinType { + pub fn layout(&self) -> SizeAlign { + match self { + BuiltinType::String => SizeAlign { size: 8, align: 4 }, // Pointer and Length + BuiltinType::U8 | BuiltinType::S8 => SizeAlign { size: 1, align: 1 }, + BuiltinType::U16 | BuiltinType::S16 => SizeAlign { size: 2, align: 2 }, + BuiltinType::U32 | BuiltinType::S32 | BuiltinType::F32 => { + SizeAlign { size: 4, align: 4 } + } + BuiltinType::U64 | BuiltinType::S64 | BuiltinType::F64 => { + SizeAlign { size: 8, align: 8 } + } + } + } +} diff --git a/tools/witx/src/lib.rs b/tools/witx/src/lib.rs index 018b55133..0e3ac3d50 100644 --- a/tools/witx/src/lib.rs +++ b/tools/witx/src/lib.rs @@ -6,6 +6,8 @@ mod coretypes; mod docs; /// Interface for filesystem or mock IO mod io; +/// Calculate memory layout of types +pub mod layout; /// Witx syntax parsing from SExprs mod parser; /// Calculate required polyfill between interfaces From 2fabc5def3edb55527a33e633f7d4cf5e0355189 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Tue, 17 Dec 2019 16:48:25 -0800 Subject: [PATCH 15/15] rustfmt --- tools/witx/src/ast.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/witx/src/ast.rs b/tools/witx/src/ast.rs index 0388673e3..224a614c6 100644 --- a/tools/witx/src/ast.rs +++ b/tools/witx/src/ast.rs @@ -68,7 +68,6 @@ impl std::hash::Hash for Document { } } - #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum Definition { Typename(Rc),