inference: fix vararg normalization in rewrap #39134
Conversation
| @@ -122,6 +122,8 @@ function ⊑(@nospecialize(a), @nospecialize(b)) | |||
| (a === Any || b === NOT_FOUND) && return false | |||
| a === Union{} && return true | |||
| b === Union{} && return false | |||
| @assert !isa(a, TypeVar) "invalid lattice item" | |||
There was a problem hiding this comment.
This function is somewhat hot. Move this check all the way down to the === check?
There was a problem hiding this comment.
I don't think a pointer comparison will affect this, relative to the many other egal and subtyping test it does
| @@ -5,7 +5,7 @@ | |||
| ##################### | |||
|
|
|||
| function rewrap(@nospecialize(t), @nospecialize(u)) | |||
| if isa(t, TypeVar) || isa(t, Type) | |||
| if isa(t, TypeVar) || isa(t, Type) || isa(t, Core.TypeofVararg) | |||
There was a problem hiding this comment.
I did add a specialization of rewrap_unionall that handles this case (since anything else would be breaking), but I'm not all that convinced that rewrap_unionall should actually work on Vararg. In many cases it's the wrong thing to do. Maybe just make the Vararg case explicit here, since this is an internal function? Alternatively, I think there's only one or two callsites of this that allow a Vararg, so maybe they should use a different entrypoint.
There was a problem hiding this comment.
I think the || already makes it pretty explicit. We can consider other designs later as continued clean up, but this seems to fix CI regression for now.
Fixes #39082