Skip to content

Allow func type param names in the field name subsection#7

Open
bvisness wants to merge 1 commit into
mainfrom
func-type-names
Open

Allow func type param names in the field name subsection#7
bvisness wants to merge 1 commit into
mainfrom
func-type-names

Conversation

@bvisness
Copy link
Copy Markdown
Collaborator

It is very natural to extend the field name subsection to include parameters for function types. The binary encoding for function types would be identical, and func types are already included under "composite types" in the spec anyway.

In this patch I have updated to keep the name "field index" and "field name subsection" because "field" is the more natural name to use in most places in the spec. Renaming everything to "composite index" felt dramatically worse to me.

Resolves #4.

It is very natural to extend the field name subsection to include
parameters for function types. The binary encoding for function types
would be identical, and func types are already included under "composite
types" in the spec anyway.

In this patch I have updated to keep the name "field index" and "field
name subsection" because "field" is the more natural name to use in most
places in the spec. Renaming everything to "composite index" felt
dramatically worse to me.
Copy link
Copy Markdown
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

@rossberg, can you check that the use of the different syntactic classes makes sense?

@bvisness
Copy link
Copy Markdown
Collaborator Author

For what it's worth, I have yet to find any tangible connection between these declared index spaces and much else in the spec. A few things in validation explicitly handle indices. Many seemingly don't, e.g. struct fields! So I dunno. I tried to make the prose straightforward and clear at least. It should be obvious what an implementer should do.

Copy link
Copy Markdown
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

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

I have yet to find any tangible connection between these declared index spaces and much else in the spec.

Do you mean in terms of how they are mapped to actual indices?

\end{array}

.. note::
All :ref:`composite types <syntax-comptype>` can be described by the field name subsection, not just :ref:`aggregate types <syntax-aggrtype>`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure I would make such a general statement, since e.g. it doesn't make much sense for array types and none for continuation types. Perhaps better enumerate the types it can apply to?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

At the time of writing, it's just structs, arrays, and functions, and the existing terminology talks about "aggregate types" instead of just struct types. So really I was just sticking with that. But I agree that it really only makes sense for structs and functions right now.

What I'm more curious about is whether it's ok for all of them to use fieldidx. To be honest, it just seems exhausting to create a different type of index for every single thing.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To be generic, you could just use plain idx (whose definition I just noticed was missing from the spec, but I just fixed that).

Label indices reference :ref:`structured control instructions <syntax-instr-control>` inside an instruction sequence. There are two index spaces for labels: one tracks the stack of active labels during validation, while the other tracks all labels defined in a specific function.

Each :ref:`aggregate type <syntax-aggrtype>` provides an index space for its :ref:`fields <syntax-fieldtype>`.
Each :ref:`composite type <syntax-comptype>` provides an index space for its :ref:`parameters <syntax-resulttype>` (for :ref:`function type <syntax-functype>`) or :ref:`fields <syntax-fieldtype>` (for :ref:`aggregate types <syntax-aggrtype>`). All such address spaces use :ref:`field indices <syntax-fieldidx>`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason not to extend this to result types? There are languages that allow naming results symmetric to parameters.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

At the moment, simply because the text format doesn't allow names for results. That said, I did think about that, and it might not be that hard to add.

The one wacky bit is that it still wouldn't make sense to give names to results in actual function declarations. WASM isn't like Go where you can name a return value and then assign to it like a variable. So what would it mean to do (func $myFunc (result $foo i32) ...instrs)? What index would the name map to?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, I guess the name section isn't really for the benefit of annotating source language names.

@bvisness
Copy link
Copy Markdown
Collaborator Author

Yes, in terms of how they are mapped to actual indices. In some cases they are explicitly attached to a context during validation, but seemingly not always (unless I'm reading it wrong). My impression is that it doesn't really matter that much, but that is why I'm playing a bit fast and loose with the terminology here. I don't see any other formalisms to extend in most cases.

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

Successfully merging this pull request may close these issues.

Support round-tripping parameter names from function type definitions

3 participants