Skip to content

Conversation

@tlively
Copy link
Member

@tlively tlively commented Nov 10, 2023

No description provided.

@tlively
Copy link
Member Author

tlively commented Nov 10, 2023

#define DELEGATE_FIELD_NAME(id, field)
#define DELEGATE_FIELD_NAME_VECTOR(id, field)
#define DELEGATE_FIELD_SCOPE_NAME_USE(id, field)
#define DELEGATE_FIELD_SCOPE_NAME_USE_VECTOR(id, field)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does the vector version do that's different?

Copy link
Member Author

Choose a reason for hiding this comment

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

br_table is the only instruction that takes a list of labels, so it is the only one that currently uses NAME_USE_VECTOR.

I'm removing the error when visiting NAME_USE because it turns out the default visitExpression implementation Just Works for BrOn, even though it includes NAME_USE. I'm making the same change for NAME_USE_VECTOR for consistency. There's nothing about having a NAME_USE_VECTOR that fundamentally means that this default implementation won't work.

return ctx.makeRefCast(pos, *type);
}

template<typename Ctx> Result<> makeBrOnNull(Ctx& ctx, Index pos, bool onFail) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm missing where the Contexts are created and how they are passed into the "make" series of functions.

Copy link
Member Author

Choose a reason for hiding this comment

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

The contexts are created and passed to the top-level parse function here: https://github.com/WebAssembly/binaryen/blob/main/src/parser/wat-parser.cpp#L96-L169

@tlively
Copy link
Member Author

tlively commented Nov 15, 2023

Merge activity

  • Nov 14, 7:40 PM: @tlively started a stack merge that includes this pull request via Graphite.
  • Nov 14, 7:43 PM: Graphite rebased this pull request as part of a merge.
  • Nov 14, 7:44 PM: @tlively merged this pull request with Graphite.

@tlively tlively force-pushed the parser-ref-test-cast branch from c08afb6 to 3d800b7 Compare November 15, 2023 00:42
Base automatically changed from parser-ref-test-cast to main November 15, 2023 00:43
@tlively tlively merged commit b01c950 into main Nov 15, 2023
@tlively tlively deleted the parser-br-on branch November 15, 2023 00:44
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
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.

3 participants