WebAssembly / component-model

Repository for design and specification of the Component Model
Other
933 stars 79 forks source link

Should method kebab names be validated in instance creation? #184

Closed alexcrichton closed 1 year ago

alexcrichton commented 1 year ago

Currently my reading of the rules is that during an instantiate instruction, and additionally during a creation of bag-of-exports, arguments with names like [method]foo.bar are validated to ensure that the first argument is (param "self" (borrow $foo)) for some $foo defined somewhere as an import/export. Should this validation be happening though? Is it instead intended that only exports/imports of a component itself get this validation?

Although as I write this down I see that bag-of-exports probably still needs at least some validation (perhaps related to https://github.com/WebAssembly/component-model/issues/183 as well), but at least for instantiation arguments it seems like the validation may wish to be relaxed (to avoid requiring the same named resources on both ends perhaps)

lukewagner commented 1 year ago

Yes, I think there should be no method-name validation happening during instantiate; the component-being-instantiated should have already validated everything relating to method names such that instantiate is just doing plain name-and-subtype matching of with arguments. In particular, the type substitution performed by instantiate to produce the resulting instance type shouldn't have any bearing on method name validation.

But for bag-of-exports, yes, I think validation is required b/c it's as-if there was a tiny utility component that imported-and-rexported everything.

alexcrichton commented 1 year ago

Should instantiate perform any validation at all of the names provided? One slight gotcha is that names can be provided that the instantiated component ignores which means that names could be provided with wild signatures and still be valid so long as the target component doesn't actually import them.

alexcrichton commented 1 year ago

Implementing this I'm realizing that bag-of-exports can't actually use method names at all. There's no index available for "this instance exports this resource" which means there's no way for later functions to refer to the resource as exported by the instance. For now that means that kebab names can't be used at all in bag-of-exports I think? The bag-of-exports construct is sort of already hanging by a thread anyway so it's perhaps not the end of the world.

lukewagner commented 1 year ago

Should instantiate perform any validation at all of the names provided?

I don't think it does; I think this name validation only kicks in when you ask "is this {component definition, component type, instance type} valid?". Thus, instantiate is only doing equality-matching of names to make sure all the imports (of a component that is already valid) are satisfied.

The bag-of-exports construct is sort of already hanging by a thread anyway so it's perhaps not the end of the world.

Just brainstorming here: it seems like now or any time later we should either nix it or generalize it (so it has its own scoped index spaces, just like nested components).

alexcrichton commented 1 year ago

So, to confirm, this should be a valid component?

(component 
  (component $C)
  (instance (instantiate $C (with "this is not kebab case" (component $C))))
)

along with this?

(component 
  (component $C)
  (instance (instantiate $C (with "[method]foo.bar" (component $C))))
)

For bag-of-exports I think we should leave it for now, but not plan on using it for anything in terms of tooling and whatnot. It seems ripe for removal in a "1.0 push" down the road.

lukewagner commented 1 year ago

The first one is not valid, but only because it's rejected by the grammar of name. But the second one is valid.


sgtm

lukewagner commented 1 year ago

I think this matches current Binary.md wording, so I'll close as resolved, but feel free to reopen if not.