Skip to content

Fix #9439: Translate Java varargs ...T into T* instead of (T & Object)* #9451

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

Merged
merged 2 commits into from
Jul 28, 2020

Conversation

smarter
Copy link
Member

@smarter smarter commented Jul 27, 2020

This is much more convenient for users but is more complicated for the
compiler since we still need to translate the varargs into an Object[]
in bytecode. Since ElimRepeated was already responsible for doing some
adaptation of repeated arguments, it now also takes care of this (this
differs from Scala 2 which handles this at Erasure).

@smarter smarter requested a review from sjrd July 27, 2020 19:55
@smarter smarter force-pushed the java-generic-varargs branch from 7dd95ad to 9d5d83e Compare July 27, 2020 20:10
@smarter smarter changed the title Fixes #9439: Translate Java varargs ...T into T* instead of (T & Object)* Fix #9439: Translate Java varargs ...T into T* instead of (T & Object)* Jul 27, 2020
Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the comments below, there's a (major) typo in the first commit message:

-we directly parse them as array, this isn't really any simpler since we
+we directly parse them as repeated params, this isn't really any simpler since we

@sjrd sjrd assigned smarter and unassigned sjrd Jul 28, 2020
smarter added 2 commits July 28, 2020 17:01
In bytecode, vararg parameters are encoded as arrays with the
ACC_VARARGS flag set on the method. Previously we first parsed them as
arrays then translated the array into a repeated param. With this commit
we directly parse them as repeated params. This isn't really any simpler
since we need to keep track of the fact that we're in a varargs method,
but it will become useful in the next commit where we start treating
arrays differently from varargs (`T[]` will still be translated as
`Array[T & Object]` whereas `T...` will be translated as `T*` and
handled specially in ElimRepeated to preserve soundness).

Similarly for joint compilation, we now directly desugar varargs as
`RepeatedParam[T]` instead of `Array[T] @Repeated`, this doesn't change
anything currently because:
- When giving a type to the method, we call `annotatedToRepeated` on
  each parameter type
- `typedAppliedTypeTree` takes care of adding `& Object` for both
  `Array[T]` and `RepeatedParam[T]` coming From Java (this is what
  the next commit will change).
This is much more convenient for users but is more complicated for the
compiler since we might still need to translate the varargs into an
`Object[]` in bytecode. Since ElimRepeated was already responsible for
doing some adaptation of repeated arguments, it now also takes care of
this (unlike Scala 2 where this is handled in Erasure).

Fixes scala#9439.
@smarter smarter force-pushed the java-generic-varargs branch from 9d5d83e to 7e68962 Compare July 28, 2020 15:03
@smarter smarter assigned sjrd and unassigned smarter Jul 28, 2020
@smarter smarter merged commit 63b84cb into scala:master Jul 28, 2020
@smarter smarter deleted the java-generic-varargs branch July 28, 2020 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants