-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix isInstanceOf[Array[?]] returning true on non-Array #12103
Conversation
Before this commit, the `MultiArrayOf(elem, ndims)` extractor used to strip wildcards from the element type, this was surprising since it does not match what the `ArrayOf(elem)` extractor does, and lead to a bug in `TypeTestCasts.interceptTypeApply` which contains the following code: case defn.MultiArrayOf(elem, ndims) if isGenericArrayElement(elem, isScala2 = false) => `isGenericArrayElement` returns false for `Any` but true for `_ >: Nothing <: Any`, so the stripped wildcard means that this case was skipped, resulting in: x.isInstanceOf[Array[?]] being erased to: x.isInstanceOf[Object] instead of: scala.runtime.ScalaRunTime.isArray(x, 1) Fixed by tweaking `MultiArrayOf` to keep any final wildcard around like `ArrayOf` does.
Milestoned to 3.0.0-RC3 since a bug in isInstanceOf seems pretty critical to me. |
val str: Any = "" | ||
assert(!str.isInstanceOf[Array[?]]) | ||
assert(!str.isInstanceOf[Array[Array[?]]]) | ||
assert(!str.isInstanceOf[Array[? <: Array[?]]]) |
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.
Also add positive tests? I.e., tests for which isInstanceOf
returns true
.
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.
That file already checks case x: Array[_]
in various situations
Before this commit, the
MultiArrayOf(elem, ndims)
extractor used tostrip wildcards from the element type, this was surprising since it does
not match what the
ArrayOf(elem)
extractor does, and lead to a bug inTypeTestCasts.interceptTypeApply
which contains the following code:isGenericArrayElement
returns false forAny
but true for_ >: Nothing <: Any
, so the stripped wildcard means that this case wasskipped, resulting in:
being erased to:
instead of:
Fixed by tweaking
MultiArrayOf
to keep any final wildcard around likeArrayOf
does.