-
Notifications
You must be signed in to change notification settings - Fork 790
Add Type comparison methods #2580
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
Conversation
| Result result = PeekType(0, &type); | ||
| if (!type.IsReferenceWithIndex()) { | ||
| type = Type::Reference; | ||
| type = Type(Type::Reference, kInvalidIndex); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good!
| } | ||
| constexpr operator Enum() const { return enum_; } | ||
|
|
||
| friend constexpr bool operator==(const Type a, const Type b) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
Thank you. Do you need more changes? |
SoniEx2
left a comment
There was a problem hiding this comment.
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!
|
The |
|
Thank you! |
Code is based on the work of SoniEx2.