Skip to content

[test] Use struct.new_desc in JS API test#96

Merged
tlively merged 1 commit intomainfrom
fix-js-prototypes-test
Feb 5, 2026
Merged

[test] Use struct.new_desc in JS API test#96
tlively merged 1 commit intomainfrom
fix-js-prototypes-test

Conversation

@tlively
Copy link
Member

@tlively tlively commented Feb 5, 2026

Update custom-descriptors/prototype.any.js to correctly use
struct.new_desc when allocating types with descriptors.

Update custom-descriptors/prototype.any.js to correctly use
struct.new_desc when allocating types with descriptors.
@tlively tlively requested a review from rmahdav February 5, 2026 01:12
kExprLocalGet, 0,
kGCPrefix, kExprStructNew, metaIndex,
kGCPrefix, kExprStructNew, descIndex,
kGCPrefix, kExprStructNewDesc, descIndex,
Copy link

Choose a reason for hiding this comment

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

Should this be descIndex or structIndex (like the similar lines)? or maybe the rest of the lines should also have descIndex instead of structIndex?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's intentional that the names are different in this test. Most of the tests have setups like this:

struct -> desc

Where the prototype is being configured on desc. But this test uses this setup instead:

struct -> desc -> meta

And the prototype is configured on meta. So the names for the base described struct and its descriptor are the same as in the other tests, but this test additionally adds meta as the descriptor of the descriptor and uses in where the previous tests would have used desc.

Copy link

Choose a reason for hiding this comment

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

Got it. thanks for the expalanation.

@tlively tlively merged commit c639c09 into main Feb 5, 2026
1 check passed
@tlively tlively deleted the fix-js-prototypes-test branch February 5, 2026 22:14
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.

2 participants