Skip to content

Add Type comparison methods#2580

Merged
SoniEx2 merged 1 commit into
WebAssembly:mainfrom
zherczeg:type_compare
Apr 3, 2025
Merged

Add Type comparison methods#2580
SoniEx2 merged 1 commit into
WebAssembly:mainfrom
zherczeg:type_compare

Conversation

@zherczeg

@zherczeg zherczeg commented Apr 3, 2025

Copy link
Copy Markdown
Collaborator

Code is based on the work of SoniEx2.

Comment thread src/type-checker.cc
Result result = PeekType(0, &type);
if (!type.IsReferenceWithIndex()) {
type = Type::Reference;
type = Type(Type::Reference, kInvalidIndex);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This was inconsistent in the code, sometimes Type(Type::Reference, kInvalidIndex) is used, sometimes just a Type::Reference. Now we must use the Type(Type::Reference, kInvalidIndex) form.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sounds good!

Comment thread include/wabt/type.h
}
constexpr operator Enum() const { return enum_; }

friend constexpr bool operator==(const Type a, const Type b) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Is Type or Type& is better?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Type is probably better, it's a pair of 32-bit values after all. Any modern compiler should be able to optimize it.

Code is based on the work of SoniEx2.
@zherczeg

zherczeg commented Apr 3, 2025

Copy link
Copy Markdown
Collaborator Author

Thank you. Do you need more changes?

@SoniEx2 SoniEx2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good thought to make Type(Type::Any) match zero-initialization. We wonder where that's being relied on (an union not being explicitly set?), but otherwise, well done!

@SoniEx2 SoniEx2 merged commit d30f043 into WebAssembly:main Apr 3, 2025
@zherczeg zherczeg deleted the type_compare branch April 3, 2025 14:41
@zherczeg

zherczeg commented Apr 3, 2025

Copy link
Copy Markdown
Collaborator Author

The T{} template constructor does it.
https://github.com/WebAssembly/wabt/blob/main/src/shared-validator.cc#L342

@zherczeg

zherczeg commented Apr 3, 2025

Copy link
Copy Markdown
Collaborator Author

Thank you!

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