Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add stream/future/error-context to wit-smith #1959

Merged
merged 1 commit into from
Dec 19, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 65 additions & 3 deletions crates/wit-component/src/printing.rs
Original file line number Diff line number Diff line change
@@ -754,9 +754,13 @@ impl<O: Output> WitPrinter<O> {
}
None => bail!("unnamed type in document"),
},
TypeDefKind::Future(_) => panic!("no need to declare futures"),
TypeDefKind::Stream(_) => panic!("no need to declare streams"),
TypeDefKind::ErrorContext => panic!("no need to declare error-contexts"),
TypeDefKind::Future(inner) => {
self.declare_future(resolve, ty.name.as_deref(), inner.as_ref())?
}
TypeDefKind::Stream(inner) => {
self.declare_stream(resolve, ty.name.as_deref(), inner)?
}
TypeDefKind::ErrorContext => self.declare_error_context(ty.name.as_deref())?,
TypeDefKind::Unknown => unreachable!(),
}
}
@@ -953,6 +957,58 @@ impl<O: Output> WitPrinter<O> {
Ok(())
}

fn declare_stream(&mut self, resolve: &Resolve, name: Option<&str>, ty: &Type) -> Result<()> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this take a non-optional name and an error be generated above if ty.name is none?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was following the precedent set by declare_list, declare_option, etc. above, which all take a Option<&str>. Should all of them be changed to non-optional? Or if not, what's different about stream?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no I didn't realize that's how everything worked. It feels weird to me to have a method that when it doesn't do anything it's silently a bug, but if it's how everything else is there's no need to change just this method.

if let Some(name) = name {
self.output.keyword("type");
self.output.str(" ");
self.print_name_type(name, TypeKind::Stream);
self.output.str(" = ");
self.output.ty("stream", TypeKind::BuiltIn);
self.output.str("<");
self.print_type_name(resolve, ty)?;
self.output.str(">");
self.output.semicolon();
}

Ok(())
}

fn declare_future(
&mut self,
resolve: &Resolve,
name: Option<&str>,
ty: Option<&Type>,
) -> Result<()> {
if let Some(name) = name {
self.output.keyword("type");
self.output.str(" ");
self.print_name_type(name, TypeKind::Future);
self.output.str(" = ");
self.output.ty("future", TypeKind::BuiltIn);
if let Some(ty) = ty {
self.output.str("<");
self.print_type_name(resolve, ty)?;
self.output.str(">");
}
self.output.semicolon();
}

Ok(())
}

fn declare_error_context(&mut self, name: Option<&str>) -> Result<()> {
if let Some(name) = name {
self.output.keyword("type");
self.output.str(" ");
self.print_name_type(name, TypeKind::ErrorContext);
self.output.str(" = ");
self.output.ty("error-context", TypeKind::BuiltIn);
self.output.semicolon();
}

Ok(())
}

fn escape_name(name: &str) -> Cow<str> {
if is_keyword(name) {
Cow::Owned(format!("%{name}"))
@@ -1212,6 +1268,8 @@ pub enum TypeKind {
BuiltIn,
/// An enumeration type name.
Enum,
/// An error-context type name.
ErrorContext,
/// A flags type name.
Flags,
/// A freestanding function name, not associated with any specific type or namespace.
@@ -1221,6 +1279,8 @@ pub enum TypeKind {
FunctionMethod,
/// A static function, associated with a resource.
FunctionStatic,
/// A future type name.
Future,
/// An interface declaration name.
InterfaceDeclaration,
/// An interface name when printing a path, for example in `use`.
@@ -1243,6 +1303,8 @@ pub enum TypeKind {
Resource,
/// A result type name.
Result,
/// A stream type name.
Stream,
/// A tuple type name.
Tuple,
/// A type alias.
28 changes: 28 additions & 0 deletions crates/wit-smith/src/generate.rs
Original file line number Diff line number Diff line change
@@ -716,6 +716,9 @@ impl<'a> InterfaceGenerator<'a> {
Option,
Result,
List,
Stream,
Future,
ErrorContext,
}

*fuel = match fuel.checked_sub(1) {
@@ -823,6 +826,31 @@ impl<'a> InterfaceGenerator<'a> {
(false, false) => {}
}
}
Kind::Stream => {
*fuel = match fuel.checked_sub(1) {
Some(fuel) => fuel,
None => continue,
};
dst.push_str("stream<");
self.gen_type(u, fuel, dst)?;
dst.push_str(">");
}
Kind::Future => {
*fuel = match fuel.checked_sub(1) {
Some(fuel) => fuel,
None => continue,
};
if u.arbitrary()? {
dst.push_str("future<");
self.gen_type(u, fuel, dst)?;
dst.push_str(">");
} else {
dst.push_str("future");
}
}
Kind::ErrorContext => {
dst.push_str("error-context");
}
};
}

7 changes: 6 additions & 1 deletion fuzz/src/wit64.rs
Original file line number Diff line number Diff line change
@@ -9,7 +9,12 @@ pub fn run(u: &mut Unstructured<'_>) -> Result<()> {
wit_smith::smith(&config, u)
})?;
write_file("doc.wasm", &wasm);
let r1 = wit_component_old::decode(&wasm).unwrap();
let Ok(r1) = wit_component_old::decode(&wasm) else {
// Presumably this is because the old version of `wit-component` doesn't
// understand the new `stream`, `future`, or `error-context` types, in
// which case there's no point in continuing, so we just return early.
return Ok(());
};
let r1 = r1.resolve();
let r2 = wit_component_new::decode(&wasm).unwrap();
let r2 = r2.resolve();