Nested interfaces in wit

[macovedj]

trafficstars

This PR enables the ability to describe nested interfaces in wit via the nest keyword.

package foo:bar;

interface baz {
  nest other:pkg/qux
}

This would indicate that that the instance associated with baz would export the instance associated with interface qux from package other:pkg.

(component 
  (type instance
    (type instance 
    ... definition of "other:pkg/qux" 
    )
   (export "other:pkg/qux" type 0)
  )
  (export "foo:bar/baz" type 0)
)

As I understand it, most use cases most immediately benefit from nesting interfaces from foreign packages, so I started with that, though we could certainly extend this PR or have follow ups to support any combo of locally defined/inlined/anonymous interfaces being nested.

This issue points out that wit currently is not able to express nested instances that can be expressed in wat/binary, so wit-bindgen can't be used by language toolchains to generate that binary. This syntax can be leveraged so that the unlocked dependency syntax referenced in the linked issue can specify the exports it expects in the imports it's describing.

[macovedj]

Sounds good! I can work on opening something up over there.

[macovedj]

I think the issues re: encoding seem like they've mostly been hashed out here. While there seems to be a plan for the non-foreign interface nesting, I think it's ok to start with what this PR originally scoped (only nest a:b/c)? There might still be a bit more conversation re: code generation, but I'll probably go ahead and try and address most of the comments here soon.

[alexcrichton]

One thing I'd also recommend for testing this is fuzzing. For example here's a small patch to enable some fuzzing of this commit:

diff --git a/crates/wit-smith/src/generate.rs b/crates/wit-smith/src/generate.rs
index 41c5c5c1..82db5fdd 100644
--- a/crates/wit-smith/src/generate.rs
+++ b/crates/wit-smith/src/generate.rs
@@ -344,6 +344,7 @@ impl<'a> InterfaceGenerator<'a> {
             Use,
             Type,
             Function,
+            Nest,
         }

         let mut parts = Vec::new();
@@ -374,6 +375,11 @@ impl<'a> InterfaceGenerator<'a> {
                 Generate::Function => {
                     parts.push(self.gen_func(u)?);
                 }
+                Generate::Nest => {
+                    if let Some(part) = self.gen_nest(u)? {
+                        parts.push(part);
+                    }
+                }
             }
         }

@@ -882,6 +888,15 @@ impl<'a> InterfaceGenerator<'a> {
     fn gen_unique_name(&mut self, u: &mut Unstructured<'_>) -> Result<String> {
         gen_unique_name(u, &mut self.unique_names)
     }
+
+    fn gen_nest(&mut self, u: &mut Unstructured<'_>) -> Result<Option<String>> {
+        let mut path = String::new();
+        if self.gen.gen_path(u, self.file, &mut path)?.is_none() {
+            return Ok(None);
+        }
+
+        Ok(Some(format!("nest {path};")))
+    }
 }

 fn gen_unique_name(u: &mut Unstructured<'_>, set: &mut HashSet<String>) -> Result<String> {

which is executed via

FUZZER=roundtrip_wit cargo +nightly fuzz run -s none run

(you can pass --dev between run and run to run in debug mode too)

That'll help weed out issues dealing with round-tripping interfaces and such. For example that quickly finds this locally for me:

thread '<unnamed>' panicked at fuzz/src/roundtrip_wit.rs:13:61:
called `Result::unwrap()` on an `Err` value: export name `name:name1/[email protected]+1.2.0` conflicts with previous name `name:name1/[email protected]+1.2.0` (at offset 0x6a)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

which I think means two things:

1. Resolve validation needs to guarantee that you can't nest the same interface twice (maybe this means it should switch to IndexSet?)
2. The fuzz generator needs to record what was nested and not generate documents that nest the same interface twice. Either that or the error is handled in the fuzzer and the test case is discarded (but I think here the former is the better route to take)

[ydnar]

Why nest instead of include?

[macovedj]

Why nest instead of include?

As I recall it, there were a few motivating factors behind this, some of which is covered in the discussion here.

1.) nest is intended to express something that is distinct from "unioning worlds" like include. Instead, if a component exports an instance, this enables one to specify expectations about the interfaces that that instance itself exports. This is described a bit here.

2.) There is the specific use case of being able to add a dependency on a specific "concrete" component (importing a specific implementation of a wit interface, as opposed to what people do currently which is import an unspecified implementation of an interface). Having the ability to do this would let you do something like cargo add for component implementations (not wit interfaces), without something like a wac step. After chatting quite a bit with @lukewagner about this, this syntax was arrived at (though could still change a bit) as a way to help accomplish this.

Re: the fact that this hasn't been touched in awhile, I wanted to get this old PR merged first, so that we can reference a wkg.lock when reasoning about the logic, but got a bit distracted, though I intend to come back to all of this.