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

Fix aggregation of imports. #67

Merged
merged 3 commits into from
Apr 7, 2024
Merged

Fix aggregation of imports. #67

merged 3 commits into from
Apr 7, 2024

Conversation

peterhuene
Copy link
Member

This PR fixes aggregation of imports that have used types.

Previously, interfaces weren't being coalesced properly, resulting in duplicate imports of used interfaces that would result in an encoding validation error.

The fix is to coalesce interfaces by interface identifier, merging interfaces together during the remapping process.

Additionally, this fixes a bug in encoding where any implicitly imported instances were not getting their encoded indexes recorded in the encoding state properly.

Also changed the map in the type aggregator to store Types rather than ItemKinds, which makes things neater.

Fixes #65.

This commit fixes aggregation of imports that have used types.

Previously, interfaces weren't being coalesced properly, resulting in duplicate
imports of used interfaces that would result in an encoding validation error.

The fix is to coalesce interfaces by interface identifier, merging interfaces
together during the remapping process.

Additionally, this fixes a bug in encoding where any implicitly imported
instances were not getting their encoded indexes recorded in the encoding state
properly.

Also changed the map in the type aggregator to store `Type`s rather than
`ItemKind`s, which makes things neater.

Fixes #65.
@peterhuene peterhuene requested a review from rylev April 5, 2024 20:27
Copy link
Collaborator

@rylev rylev left a comment

Choose a reason for hiding this comment

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

Just a few tiny nits. Looks great!

crates/wac-types/src/aggregator.rs Show resolved Hide resolved
crates/wac-types/src/aggregator.rs Outdated Show resolved Hide resolved
crates/wac-types/src/aggregator.rs Outdated Show resolved Hide resolved
crates/wac-types/src/aggregator.rs Outdated Show resolved Hide resolved
@peterhuene peterhuene merged commit f398c13 into main Apr 7, 2024
7 checks passed
@peterhuene peterhuene deleted the fix-aggregation branch April 7, 2024 18:46
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.

[Graph] error encoding a component: "import name conflicts with previous name"
2 participants