Fix stringview subtyping #6440
Fix stringview subtyping #6440
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| return a; | ||
| } | ||
| if (HeapType(a).getBottom() != HeapType(b).getBottom()) { | ||
| if (HeapType(a).getTop() != HeapType(b).getTop()) { |
There was a problem hiding this comment.
Will we not have many other places like this? This worries me...
How bad are things if we don't do this? I may be missing that part.
There was a problem hiding this comment.
Without this change, we would accept (or produce, in the case of TypeGeneralizing) modules that have stringviews flowing into anyref locations, which V8 would reject. GUFA could also produce casts to stringview types for such modules, which V8 would also reject.
There was a problem hiding this comment.
Accepting such inputs worries me less as we can say that is user error (and we don't expect to actually see it, nor have we). For GUFA I think we have a fix, and for TypeGeneralizing it should be simple as well, I think? I guess I have a clearer idea of the downsides of those workarounds than such a fundamental change to the type system.
There was a problem hiding this comment.
I'd rather fix the stringref spec, but since we can't, it seems best to at least implement stringref correctly. That seems much better to me than having to special case strings in various places because we haven't implemented their types correctly.
The stringview types (`stringview_wtf8`, `stringview_wtf16`, and `stringview_iter`) are not subtypes of `any` even though they are supertypes of `none`. This breaks the type system invariant that types share a bottom type iff they share a top type, but we can work around that.
tlively
force-pushed
the
fix-stringview-typing
branch
from
c9c359f to
4f808e0
Compare
|
Oops, I have some stray changes in here. |
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
lgtm with fuzzing. I suggest also filing a stringref issue for posterity.
|
Fuzzer is still going after 20k+ iterations, so I'm going to call it good enough. |
The stringview types (
stringview_wtf8,stringview_wtf16, andstringview_iter) are not subtypes ofanyeven though they are supertypes ofnone. This breaks the type system invariant that types share a bottom type iffthey share a top type, but we can work around that.