Skip to content

Use ImportResolver for tag imports#8254

Open
stevenfontanella wants to merge 8 commits intomainfrom
tag-import
Open

Use ImportResolver for tag imports#8254
stevenfontanella wants to merge 8 commits intomainfrom
tag-import

Conversation

@stevenfontanella
Copy link
Member

@stevenfontanella stevenfontanella commented Jan 29, 2026

Part of #8180 and #8261. Fixes the semantics/spec test when the same tag is imported in different instances, in which case the tag should behave as a new identity, which was previously not the case (see the tags in the modified instance.wast in this PR).

@stevenfontanella stevenfontanella changed the title Refactor tag / tag import logic Use ImportResolver for tag imports Feb 3, 2026
@stevenfontanella stevenfontanella marked this pull request as ready for review February 3, 2026 04:43
// internal name.
std::vector<Literals> definedGlobals;
std::vector<std::unique_ptr<RuntimeTable>> definedTables;
std::vector<Tag> definedTags;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need std::unique_ptr for definedTables but not for definedGLobals or definedTags? Or could we get rid of the unique ptr in a separate change?

Copy link
Member Author

Choose a reason for hiding this comment

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

definedTables are unique_ptrs because RuntimeTable is a virtual type. It also makes a little more sense for tables than for globals and tags since tables are more like 'control' while globals and tags are more like plain data (setting aside what we discussed about tags having an identity).

We could potentially remove the indirection by changing it to RealRuntimeTable but it isn't possible right now because we inject a fake RuntimeTable in ctor eval: link. Once we resolve the TODO mentioned there, we could change it. On that note: what's the reasoning for wanting to remove the unique_ptr here? Is it speed or something else?

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, thanks. My motivation for asking is that I think it would be nice to be as uniform as possible with how we treat the different kinds of module fields and their corresponding runtime objects here. So I would have expected the vector of defined tags to look very much like the vector of defined tables, i.e. both or neither using unique_ptrs. (This is also why I would expect the tags to be represented by RuntimeTag objects containing pointers to the instantiated Tag definitions, just like RuntimeTable has a pointer to a Table.)

I would go as far as to say that RuntimeTag doesn't need to support an equality operator or do anything except wrap a Tag*. We can tell if two RuntimeTags are the same if they physically have the same address.

Copy link
Member Author

Choose a reason for hiding this comment

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

So is you point that because these are all module fields / importable things that they should all either be unique_ptrs or not unique_ptrs? I feel that they have that in common, but what they don't have in common is whether they encode data or behavior. Literals are closer to data and tables/memories/functions are more of behavior. e.g. for tables we disable copy construction and move assignment, but it's fine to do this with a Literal (and we do do that here). Also it's a little strange (although allowed) to do copy assignment on the pointed-at value of a unique_ptr, which the former code link would do if we changed definedGlobals to hold unique_ptrs.

The rule I'm following is that value objects can be held directly, while behavior objects disable copying/assignment and are held with unique_ptrs. From that perspective it's fine if globals aren't held in a unique_ptr.

I didn't mention tags because they're in an in-between state in this PR. Here I'm using a 'data' object to represent them, but it would mostly be wrong to copy them since it would change its identity. In the next PR I plan to add a type for them that can't be copied, or maybe I'd represent them with unique_ptrs instead.

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