Uh oh!
There was an error while loading. Please reload this page.
Conversation
tlively
left a comment
There was a problem hiding this comment.
This LGTM % some small comments. It would be very exciting if this let us simplify the validator.
| // example, | ||
| // in a CallIndirect. |
There was a problem hiding this comment.
Looks like the formatting could be more consistent here.
| // * noteCast(Expression, Expression) - An expression's type is cast to | ||
| // another, for example, in RefCast. |
There was a problem hiding this comment.
Perhaps I'll find out below, but what is the benefit of this over just extracting the type of the cast expression and using noteCast(Expression, Type) in this case?
There was a problem hiding this comment.
This PR tries to keep the info as "high-level" as possible, that is, not to lose information. We might want the extra information here to say something about the expression that fails to validate, if we use this in the validator some day (though, if we have the cast anyhow, we can do cast->value).
| // The parent must also inherit from ControlFlowWalker (for findBreakTarget). | ||
| // | ||
| template<typename Parent> |
There was a problem hiding this comment.
Do we have any other cases where we take a parent as a parameter instead of a subclass as a parameter?
There was a problem hiding this comment.
Oh, maybe "Parent" is a confusing name here, yeah. It's the same as the usual pattern.
Uh oh!
There was an error while loading. Please reload this page.
tlively
commented
Nov 14, 2023
I think that if we wanted to use this in the validator, we would want to note the "subtyping" for non-ref values as well. |
kripken
commented
Nov 14, 2023
Good point, yeah - maybe when we get there, this could be renamed to ReferenceSubtypingDiscoverer, and we'd have a subclass that is a general one that adds non-refs? |
This implements an idea I mentioned in the past, to extract the subtyping discovery code out of Unsubtyping so it could be reused elsewhere. Example possible uses: the validator could use to remove a lot of code, and also a future PR of mine will need it. Separately from those, I think this is a nice refactoring as it makes Unsubtyping much smaller. This just moves the code out and adds some C++ template elbow grease as needed.
This implements an idea I mentioned in the past, to extract the subtyping discovery
code out of Unsubtyping so it could be reused elsewhere. Example possible uses:
the validator could use to remove a lot of code, and also a future PR of mine will
need it. Separately from those, I think this is a nice refactoring as it makes Unsubtyping
much smaller.
This just moves the code out and adds some C++ template elbow grease as needed.