Skip to content

Commit

Permalink
Change data representation of resource methods (#1070)
Browse files Browse the repository at this point in the history
* Change data representation of resource methods

This commit changes the representation of resources and their methods in
the `wit-parser` crate as implemented originally in #1053. Previously
methods related to a resource were strung as a separate list of methods
from each `Resource` type which provided a nested view of functions for
bindings generators to interpret. This representation, however, does not
allow for methods or constructors to actually refer to the resource type
itself because the resource type can't be created until the method list
is created, a cyclic dependency.

To handle this problem this commit removes the nested list of functions
and instead places all resource methods and functions and such within
the normal single list of functions within an interface. This represents
a "flattened" version of all functions available within an interface or
world and aligns with the WebAssembly representation of how these will
work. The names of these functions are not valid identifiers and have
the component-model mangling along the lines of `[method]foo.bar` for
example.

My hope is that in the future as bindings generators are worked on
various helper methods can be added to automatically group functions
based on their kind to avoid any issues. Otherwise though this will
likely "break" all existing code generators when resources are
introduced because the names aren't valid identifiers in any language.
I think this is a good thing, however, as that way it can't be forgotten
to handle the resources case and failures will be "loud" in theory.

This commit changes a number of other miscellaneous details such as:

* Type resolution no longer needs to consider types referenced by
  methods of resources as dependencies which reflects how this will all
  look in a wasm binary format where resources are declared before their
  member functions are declared.

* Parsing has been updated to address some minor issues here and there
  and additionally has some new names to remove the old "value" concept
  which is no longer a thing.

* Fix resources-in-worlds with methods

Additionally be sure to update ids listed in `FunctionKind` during
merging.
  • Loading branch information
alexcrichton authored Jun 16, 2023
1 parent 2180971 commit 4b69456
Show file tree
Hide file tree
Showing 23 changed files with 391 additions and 298 deletions.
2 changes: 1 addition & 1 deletion crates/wit-component/src/decoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ impl WitPackageDecoder<'_> {
| TypeDefKind::Result(_)
| TypeDefKind::Handle(_) => {}

TypeDefKind::Resource(_)
TypeDefKind::Resource
| TypeDefKind::Record(_)
| TypeDefKind::Enum(_)
| TypeDefKind::Variant(_)
Expand Down
2 changes: 1 addition & 1 deletion crates/wit-component/src/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ impl TypeContents {
TypeDefKind::Handle(h) => match h {
wit_parser::Handle::Shared(_) => Self::empty(),
},
TypeDefKind::Resource(..) => Self::empty(),
TypeDefKind::Resource => Self::empty(),
TypeDefKind::Record(r) => Self::for_types(resolve, r.fields.iter().map(|f| &f.ty)),
TypeDefKind::Tuple(t) => Self::for_types(resolve, t.types.iter()),
TypeDefKind::Flags(_) => Self::empty(),
Expand Down
2 changes: 1 addition & 1 deletion crates/wit-component/src/encoding/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ pub trait ValtypeEncoder<'a> {
TypeDefKind::Future(_) => todo!("encoding for future type"),
TypeDefKind::Stream(_) => todo!("encoding for stream type"),
TypeDefKind::Unknown => unreachable!(),
TypeDefKind::Resource(_) => todo!(),
TypeDefKind::Resource => todo!(),
TypeDefKind::Handle(_) => todo!(),
};

Expand Down
80 changes: 4 additions & 76 deletions crates/wit-component/src/printing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl WitPrinter {
TypeDefKind::Handle(h) => {
self.print_handle_type(resolve, h)?;
}
TypeDefKind::Resource(_) => {
TypeDefKind::Resource => {
bail!("resolve has an unnamed resource type");
}
TypeDefKind::Tuple(t) => {
Expand Down Expand Up @@ -457,9 +457,7 @@ impl WitPrinter {
TypeDefKind::Handle(h) => {
self.declare_handle(resolve, ty.name.as_deref(), h)?
}
TypeDefKind::Resource(r) => {
self.declare_resource(resolve, ty.name.as_deref(), r)?
}
TypeDefKind::Resource => self.declare_resource(resolve, ty.name.as_deref())?,
TypeDefKind::Record(r) => {
self.declare_record(resolve, ty.name.as_deref(), r)?
}
Expand Down Expand Up @@ -517,88 +515,18 @@ impl WitPrinter {
}
}

fn declare_resource(
&mut self,
resolve: &Resolve,
name: Option<&str>,
resource: &Resource,
) -> Result<()> {
fn declare_resource(&mut self, _resolve: &Resolve, name: Option<&str>) -> Result<()> {
match name {
Some(name) => {
self.output.push_str("resource ");
self.print_name(name);
self.output.push_str(" {\n");
for function in &resource.methods {
self.declare_function(resolve, Some(name), function)?;
}
self.output.push_str(" }\n\n");
self.output.push_str("\n\n");
Ok(())
}
None => bail!("document has unnamed resource type"),
}
}

fn declare_function(
&mut self,
resolve: &Resolve,
name: Option<&str>,
function: &Function,
) -> Result<()> {
match function.kind {
FunctionKind::Static => {
self.output.push_str("static ");
}
_ => {}
}

self.print_name(&function.name);
self.output.push_str(": func(");

match function.kind {
FunctionKind::Method => match name {
Some(name) => {
self.output.push_str(&format!("self: shared<{name}>"));
}
None => bail!("document has unnamed resource type"),
},
_ => {}
}

for (name, ty) in &function.params {
self.print_name(&name);
self.output.push_str(": ");
self.print_type_name(resolve, &ty)?;
self.output.push_str(", ");
}

self.output.push_str(") ");

self.output.push_str("-> ");

match &function.results {
Results::Named(results) => {
self.output.push_str("(");

if results.len() > 0 {
for (name, ty) in results {
self.print_name(&name);
self.output.push_str(": ");
self.print_type_name(resolve, &ty)?;
self.output.push_str(", ");
}
}
self.output.push_str(")");
}
Results::Anon(ty) => {
self.print_type_name(resolve, &ty)?;
}
}

self.output.push_str(";\n");

Ok(())
}

fn declare_record(
&mut self,
resolve: &Resolve,
Expand Down
14 changes: 7 additions & 7 deletions crates/wit-parser/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ impl Resolve {

TypeDefKind::Handle(_) => todo!(),

TypeDefKind::Resource(_) => todo!(),
TypeDefKind::Resource => todo!(),

TypeDefKind::Record(r) => {
for field in r.fields.iter() {
Expand Down Expand Up @@ -904,7 +904,7 @@ impl Resolve {
TypeDefKind::List(_) => true,
TypeDefKind::Type(t) => self.needs_post_return(t),
TypeDefKind::Handle(_) => true,
TypeDefKind::Resource(_) => true,
TypeDefKind::Resource => true,
TypeDefKind::Record(r) => r.fields.iter().any(|f| self.needs_post_return(&f.ty)),
TypeDefKind::Tuple(t) => t.types.iter().any(|t| self.needs_post_return(t)),
TypeDefKind::Union(t) => t.cases.iter().any(|t| self.needs_post_return(&t.ty)),
Expand Down Expand Up @@ -1292,7 +1292,7 @@ impl<'a, B: Bindgen> Generator<'a, B> {
}
}
TypeDefKind::Handle(_) => todo!(),
TypeDefKind::Resource(_) => {
TypeDefKind::Resource => {
todo!();
}
TypeDefKind::Record(record) => {
Expand Down Expand Up @@ -1481,7 +1481,7 @@ impl<'a, B: Bindgen> Generator<'a, B> {
TypeDefKind::Handle(_) => {
todo!();
}
TypeDefKind::Resource(_) => {
TypeDefKind::Resource => {
todo!();
}
TypeDefKind::Record(record) => {
Expand Down Expand Up @@ -1643,7 +1643,7 @@ impl<'a, B: Bindgen> Generator<'a, B> {
});
self.write_fields_to_memory(record.fields.iter().map(|f| &f.ty), addr, offset);
}
TypeDefKind::Resource(_) => {
TypeDefKind::Resource => {
todo!()
}
TypeDefKind::Tuple(tuple) => {
Expand Down Expand Up @@ -1839,7 +1839,7 @@ impl<'a, B: Bindgen> Generator<'a, B> {
todo!();
}

TypeDefKind::Resource(_) => {
TypeDefKind::Resource => {
todo!();
}

Expand Down Expand Up @@ -2063,7 +2063,7 @@ impl<'a, B: Bindgen> Generator<'a, B> {
todo!()
}

TypeDefKind::Resource(_) => {
TypeDefKind::Resource => {
todo!()
}

Expand Down
123 changes: 68 additions & 55 deletions crates/wit-parser/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ pub enum WorldOrInterface {

enum InterfaceItem<'a> {
TypeDef(TypeDef<'a>),
Value(Value<'a>),
Func(NamedFunc<'a>),
Use(Use<'a>),
}

Expand Down Expand Up @@ -543,7 +543,58 @@ enum Handle<'a> {
}

struct Resource<'a> {
methods: Vec<Value<'a>>,
funcs: Vec<ResourceFunc<'a>>,
}

enum ResourceFunc<'a> {
Method(NamedFunc<'a>),
Static(NamedFunc<'a>),
Constructor(NamedFunc<'a>),
}

impl<'a> ResourceFunc<'a> {
fn parse(docs: Docs<'a>, tokens: &mut Tokenizer<'a>) -> Result<Self> {
match tokens.clone().next()? {
Some((_span, Token::Static)) => {
tokens.expect(Token::Static)?;
Ok(ResourceFunc::Static(NamedFunc::parse(tokens, docs)?))
}
Some((span, Token::Constructor)) => {
tokens.expect(Token::Constructor)?;
tokens.expect(Token::LeftParen)?;
let params = parse_list_trailer(tokens, Token::RightParen, |_docs, tokens| {
let name = parse_id(tokens)?;
tokens.expect(Token::Colon)?;
let ty = Type::parse(tokens)?;
Ok((name, ty))
})?;
Ok(ResourceFunc::Constructor(NamedFunc {
docs,
name: Id {
span,
name: "constructor",
},
func: Func {
params,
results: ResultList::Named(Vec::new()),
},
}))
}
Some((_span, Token::Id | Token::ExplicitId)) => {
Ok(ResourceFunc::Method(NamedFunc::parse(tokens, docs)?))
}
other => {
Err(err_expected(tokens, "`static`, `constructor` or identifier", other).into())
}
}
}

fn named_func(&self) -> &NamedFunc<'a> {
use ResourceFunc::*;
match self {
Method(f) | Static(f) | Constructor(f) => f,
}
}
}

struct Record<'a> {
Expand Down Expand Up @@ -596,10 +647,10 @@ struct Stream<'a> {
end: Option<Box<Type<'a>>>,
}

struct Value<'a> {
struct NamedFunc<'a> {
docs: Docs<'a>,
name: Id<'a>,
kind: ValueKind<'a>,
func: Func<'a>,
}

struct Union<'a> {
Expand All @@ -619,11 +670,6 @@ enum ResultList<'a> {
Anon(Type<'a>),
}

enum ValueKind<'a> {
Func(Func<'a>),
Static(Func<'a>),
}

struct Func<'a> {
params: ParamList<'a>,
results: ResultList<'a>,
Expand Down Expand Up @@ -662,15 +708,6 @@ impl<'a> Func<'a> {
}
}

impl<'a> ValueKind<'a> {
fn parse_func(tokens: &mut Tokenizer<'a>) -> Result<ValueKind<'a>> {
Func::parse(tokens).map(ValueKind::Func)
}
fn parse_static(tokens: &mut Tokenizer<'a>) -> Result<ValueKind<'a>> {
Func::parse(tokens).map(ValueKind::Static)
}
}

impl<'a> InterfaceItem<'a> {
fn parse(tokens: &mut Tokenizer<'a>, docs: Docs<'a>) -> Result<InterfaceItem<'a>> {
match tokens.clone().next()? {
Expand All @@ -694,7 +731,7 @@ impl<'a> InterfaceItem<'a> {
TypeDef::parse_union(tokens, docs).map(InterfaceItem::TypeDef)
}
Some((_span, Token::Id)) | Some((_span, Token::ExplicitId)) => {
Value::parse(tokens, docs).map(InterfaceItem::Value)
NamedFunc::parse(tokens, docs).map(InterfaceItem::Func)
}
Some((_span, Token::Use)) => Use::parse(tokens).map(InterfaceItem::Use),
other => Err(err_expected(tokens, "`type`, `resource` or `func`", other).into()),
Expand Down Expand Up @@ -731,14 +768,13 @@ impl<'a> TypeDef<'a> {
fn parse_resource(tokens: &mut Tokenizer<'a>, docs: Docs<'a>) -> Result<Self> {
tokens.expect(Token::Resource)?;
let name = parse_id(tokens)?;
let ty = Type::Resource(Resource {
methods: parse_list(
tokens,
Token::LeftBrace,
Token::RightBrace,
|docs, tokens| Ok(Value::parse(tokens, docs)?),
)?,
});
let mut funcs = Vec::new();
if tokens.eat(Token::LeftBrace)? {
while !tokens.eat(Token::RightBrace)? {
funcs.push(ResourceFunc::parse(parse_docs(tokens)?, tokens)?);
}
}
let ty = Type::Resource(Resource { funcs });
Ok(TypeDef { docs, name, ty })
}

Expand Down Expand Up @@ -823,35 +859,12 @@ impl<'a> TypeDef<'a> {
}
}

impl<'a> Value<'a> {
impl<'a> NamedFunc<'a> {
fn parse(tokens: &mut Tokenizer<'a>, docs: Docs<'a>) -> Result<Self> {
let name = match tokens.next()? {
Some((_, Token::Static)) => None,
Some((span, Token::Id)) => Some(Id {
name: tokens.parse_id(span)?,
span,
}),
Some((span, Token::ExplicitId)) => Some(Id {
name: tokens.parse_explicit_id(span)?,
span,
}),
other => Err(err_expected(tokens, "static or an identifier", other))?,
};

let (name, kind) = match name {
Some(name) => {
tokens.expect(Token::Colon)?;
let kind = ValueKind::parse_func(tokens)?;
(name, kind)
}
None => {
let name = parse_id(tokens)?;
tokens.expect(Token::Colon)?;
let kind = ValueKind::parse_static(tokens)?;
(name, kind)
}
};
Ok(Value { docs, name, kind })
let name = parse_id(tokens)?;
tokens.expect(Token::Colon)?;
let func = Func::parse(tokens)?;
Ok(NamedFunc { docs, name, func })
}
}

Expand Down
Loading

0 comments on commit 4b69456

Please sign in to comment.