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

Rename import in thee case where it isn't an import #2

Closed
sunfishcode opened this issue Feb 22, 2022 · 7 comments
Closed

Rename import in thee case where it isn't an import #2

sunfishcode opened this issue Feb 22, 2022 · 7 comments

Comments

@sunfishcode
Copy link
Member

In the explainer:

instanceexpr ::= (instantiate (module ) (import ))
| (instantiate (component ) (import )
)
| (instance *)

Could we rename the import nodes to with? Or modulearg/componentarg, or so? (import ...) has a different meaning in a different context. Parsers can disambiguate it, but it's confusing for humans that don't already know their way around.

For example:

instanceexpr ::= (instantiate (module <moduleidx>) (with <name> <modulearg>)*)
               | (instantiate (component <componentidx>) (with <name> <componentarg>)*)
               | (instance <export>*)
@lukewagner
Copy link
Member

I'm open to collecting more feedback to see if other folks would prefer with to import. The current naming is inherited from the module-linking proposal since around February 2021.

@sunfishcode
Copy link
Member Author

import is reused for componentargs being passed to instantiate. export is reused for componentargs being bundled into an instantiation record. instance is reused for instantiation records.

People already familiar with the language may not find these difficult to navigate. But my experience as someone familiar with what module linking is doing, and familiar with core-wasm syntax, but not deeply familiar with module-linking syntax, is that this language is already really abstract and subtle. Some of this is difficult to avoid, such as core-wasm vs. component differences, and the overloading of the leading syntax for "a literal thing", "reference a thing", and "type of a thing". But each additional multi-use keyword on top of that compounds with those. It gets even harder to look up a keyword in the documentation, harder to find other examples using that keyword in the same way, and harder to find one's bearings when jumping into the middle of a piece of code.

@lukewagner
Copy link
Member

That's a fair point, especially with the more-recent addition of the inline instances, where now you have both import and export tokens nestled inside an instantiate expression and serving very different, asymmetric roles. I'm inclined to merge #7 into #6, but I'll leave it up for a while to collect feedback.

@rossberg
Copy link
Member

rossberg commented Mar 9, 2022

I agree that the amount of notational overloading is a bit confusing. However, I think arg would be an odd choice, given that we do not call imports "parameters". Ideally, it should be something that either matches or is a kind of counterpart to "import" (perhaps "provide"?). But I guess something neutral like "with" would be fine as well.

@sunfishcode
Copy link
Member Author

with works for me; I've now updated the PR to use with instead of arg.

That said, this very document does call these things "arguments":

Instance definitions create module or component instances by selecting a module/component and supplying a set of named arguments which satisfy all the named imports of the selected module/component

@lukewagner
Copy link
Member

with sgtm2

@lukewagner
Copy link
Member

Updated, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants