Fix #5720: don't output X <: Long in Java generic signatures #5816
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
assigned 
Blaisorblade
force-pushed
the
fix-5720-higher-kind-erasure
branch
from
944e698 to
01f16a0
Compare
smarter
reviewed
Jan 29, 2019
This comment has been minimized.
This comment has been minimized.
|
Still todo: maybe move tests/fuzzy/IAE-4a69457e1319217c3bac170110ea4ba58dca11a6.scala to |
This comment has been minimized.
This comment has been minimized.
Blaisorblade
force-pushed
the
fix-5720-higher-kind-erasure
branch
2 times, most recently
from
5901575 to
1fc9b1b
Compare
|
@biboudis Your bot keeps assigning people to PR, can you make it stop ? |
nicolasstucki
previously requested changes
Jan 30, 2019
tests/pos/i5720.scala
Outdated
| @@ -0,0 +1,5 @@ | |||
| class A[B[_] <: Long] | |||
There was a problem hiding this comment.
I would also add some tests in the form of
class A[B[_] <: Foo]
class Foo(val i: Int) extends AnyValThere was a problem hiding this comment.
And with an opaque type that has a primitive type
There was a problem hiding this comment.
We should also have tests for all primitives
class AByte[B[_] <: Byte]
...
class AInt[B[_] <: Int]
...
|
The fix LGTM |
|
@smarter looking into it! |
In generic signature generator `jsig`, flag `primitiveOK` remembers if `jsig` is allowed to output primitive types or not; for instance, upper bounds can't contain primitive types. When transforming higher-kinded upper bounds `[X] => T`, `jsig` correctly recurs on `T`, but must remember flag `primitiveOK`. The status of `toplevel` should not change either (since it tracks if are still at the toplevel of the output signature, or instead we have started producing output), so passing it seems safest. Ideally we should test it at least on higher types whose body is a suitable `ClassInfo`; with polymorphic function types, we should also test `PolyType` bodies. Revised with tests requested in review.
Blaisorblade
force-pushed
the
fix-5720-higher-kind-erasure
branch
from
81a005c to
c954ef0
Compare
Blaisorblade
dismissed
nicolasstucki’s stale review
Added requested tests :-)
nicolasstucki
approved these changes
Jan 30, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In generic signature generator
jsig, flagprimitiveOKremembers ifjsigisallowed to output primitive types or not; for instance, upper bounds can't
contain primitive types. When transforming higher-kinded upper bounds
[X] => T,jsigcorrectly recurs onT, but must remember flagprimitiveOK.The status of
toplevelshould not change either (since it tracks if are stillat the toplevel of the output signature, or instead we have started producing
output), so passing it seems safest. Ideally we should test it at least on
higher types whose body is a suitable
ClassInfo; with polymorphic functiontypes, we should also test
PolyTypebodies.Fix #5720.