Skip to content
This repository has been archived by the owner on Jul 12, 2024. It is now read-only.

Take inherited fields into account. #22

Merged
merged 4 commits into from
Mar 19, 2024

Conversation

sjrd
Copy link
Collaborator

@sjrd sjrd commented Mar 17, 2024

In LinkedClasses, the fields only list fields declared directly in the given class. When allocating Wasm structs and computing indices, we must take inherited fields into account as well.

Based on #21.

@sjrd sjrd requested a review from tanishiking March 17, 2024 15:11
sjrd added 4 commits March 18, 2024 18:04
Now that we support boxing and unboxing of primitives, we can use
the original definitions instead of patching them.
This will make sure that we do not confuse static and non-static
methods with the same signatures.
In `LinkedClass`es, the `fields` only list fields declared directly
in the given class. When allocating Wasm structs and computing
indices, we must take inherited fields into account as well.
@sjrd sjrd force-pushed the inherited-fields branch from 7f4ce51 to 8746b6b Compare March 18, 2024 17:09
@tanishiking tanishiking marked this pull request as ready for review March 19, 2024 04:04
Copy link
Owner

@tanishiking tanishiking left a comment

Choose a reason for hiding this comment

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

Looking at 8746b6b it looks good to me, thank you for fixing this!

// Sort for stability
val sortedClasses = onlyModule.classDefs.sortBy(_.className)
/* Sort by ancestor count so that superclasses always appear before
* subclasses, then tie-break by name for stability.
Copy link
Owner

Choose a reason for hiding this comment

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

👍

@tanishiking tanishiking merged commit 26a1425 into tanishiking:main Mar 19, 2024
1 check passed
@sjrd sjrd deleted the inherited-fields branch March 19, 2024 06:25
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants