Skip to content

Conversation

tlively
Copy link
Member

@tlively tlively commented Jun 24, 2024

Add spec tests checking validation for structs and arrays.

@tlively tlively requested a review from kripken June 24, 2024 21:47
Add spec tests checking validation for structs and arrays.
break;
case HeapTypeInfo::StructKind:
for (auto& field : info.struct_.fields) {
if (field.type.isRef() && !field.type.getHeapType().isShared()) {
Copy link
Member

Choose a reason for hiding this comment

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

It seems surprising this only applies to references, when we have shared Memories for example, so we do have shared i32s etc. But I guess they must be referred to indirectly?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we don't have separate shared i32 as a type. i32 and other non-reference types validate just fine in both shared and unshared contexts.

src/wasm-type.h Outdated
// A continuation reference that does not refer to a function type.
InvalidFuncType,
// A non-shared field of a shared heap type.
InvalidFieldType,
Copy link
Member

Choose a reason for hiding this comment

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

This could perhaps be more specific? How about UnsharedFieldOfShared?

Copy link
Member Author

Choose a reason for hiding this comment

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

I went with InvalidUnsharedField. I think that's more similar to existing error names.

@tlively tlively force-pushed the shared-validation branch from 4bd773a to 1722d14 Compare June 25, 2024 02:04
@tlively tlively merged commit 4cd8b61 into main Jun 25, 2024
@tlively tlively deleted the shared-validation branch June 25, 2024 16:35
@gkdn gkdn mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants