Skip to content
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

Work around limitation for JDK12+ about j.l.constant.Constable #1941

Merged
merged 1 commit into from Nov 5, 2020

Conversation

catap
Copy link
Contributor

@catap catap commented Oct 10, 2020

java.lang.Constable was introduced at JDK12 and it requires to link an application by scala-native via JDK12+.

It oversteps #1938

@catap catap force-pushed the jdk12 branch 3 times, most recently from 2a1110e to 08433f5 Compare October 11, 2020 09:00
@sjrd
Copy link
Collaborator

sjrd commented Oct 12, 2020

If we're going that route, we should also add ConstantDesc, which can in theory expose the same issue.

However, I'm not a fan of adding those, because we will have to leave their methods undeclared, given that we cannot implement them. We're only pushing problems a bit further.

Can we fix the test suite so that those types are not inferred instead?

@catap
Copy link
Contributor Author

catap commented Oct 13, 2020

@sjrd yes, you raised a very good point.

Let split it to two separated topic:

  1. fix building tests on JDK12+ and let me fix it by changing unit tests via this PR;
  2. address an issue at least via Can't build expression on scala-native via JDK12+ #1938

@catap catap changed the title Introduced java.lang.constant.Constable for jdk12+ Overstep limitation for JDK12+ Oct 13, 2020
@catap catap changed the title Overstep limitation for JDK12+ Overstep limitation for JDK12+ at unit-tests Oct 13, 2020
@ekrich
Copy link
Member

ekrich commented Oct 14, 2020

I could just add these changes to the JUnit PR so I don't have to rebase these.

@ekrich
Copy link
Member

ekrich commented Oct 14, 2020

Gosh, I was trying the latest with AdoptOpenJDK 15 and got this.

[info] Compiling 542 Scala sources to /Users/eric/workspace/scala-native/scalalib/target/scala-2.11/classes ...
[error] /Users/eric/workspace/scala-native/scalalib/target/scalaSources/2.11.12/scala/collection/mutable/StringBuilder.scala:31:13: overriding method isEmpty in trait CharSequence of type ()Boolean;
[error]  method isEmpty in trait IndexedSeqOptimized of type => Boolean cannot override a concrete member without a third member that's overridden by both (this rule is designed to prevent ``accidental overrides'')
[error] final class StringBuilder(private val underlying: JavaStringBuilder)
[error]             ^
[error] one error found

@catap
Copy link
Contributor Author

catap commented Oct 14, 2020

@errikos yes, scala doesn't compatible with JDK15+.

You can use this branch that fixes the issue: scala/scala#9239

@catap
Copy link
Contributor Author

catap commented Oct 20, 2020

@sjrd let me rebase this branch

@ekrich
Copy link
Member

ekrich commented Oct 20, 2020

@catap Thanks for being patient with your change. Not sure that the second file got rebased correctly.

`java.lang.Constable` was introduced at JDK12 and simple case:
```
Seq(Double.box(1d), "")
```
has type such as `Seq[java.lang.constant.Constable with ...]`

Because Scala-Native hasn't got `java.lang.constant.Constable` and it is
impossibly to implement it forces build of test to fail.

This way is allow to fix build by forces compiler to use `Object`.
@catap
Copy link
Contributor Author

catap commented Nov 4, 2020

@sjrd maybe move forward on it?

@sjrd sjrd changed the title Overstep limitation for JDK12+ at unit-tests Work around limitation for JDK12+ about j.l.Constable Nov 5, 2020
@sjrd sjrd changed the title Work around limitation for JDK12+ about j.l.Constable Work around limitation for JDK12+ about j.l.constant.Constable Nov 5, 2020
@sjrd sjrd merged commit b0ab3fb into scala-native:master Nov 5, 2020
1 check passed
@catap catap deleted the jdk12 branch November 5, 2020 11:09
vicopem pushed a commit to vicopem/scala-native that referenced this pull request Nov 19, 2020
…-native#1941)

`java.lang.constant.Constable` was introduced at JDK12 and is a
super trait of core boxed classes. This causes code like

    val seq = Seq(Double.box(1d), "")
    val elem = seq.head

to infer types that mention `Constable`: `seq` is of type
`Seq[Constable with ...]` and `elem` of type `Constable with ...`.
The latter erases to `Constable`, which results in linking errors.

It is unfortunately impossible to correctly implement `Constable`
in Scala Native, because it requires reflection, so the general
issue is not fixable.

This commit works around the issue in our own test suite, by
ascribing some values with explicit types, so that `Constable` does
not appear in erased types.
vicopem added a commit to vicopem/scala-native that referenced this pull request Nov 19, 2020
vicopem added a commit to vicopem/scala-native that referenced this pull request Nov 19, 2020
ekrich pushed a commit to ekrich/scala-native that referenced this pull request May 21, 2021
…-native#1941)

`java.lang.constant.Constable` was introduced at JDK12 and is a
super trait of core boxed classes. This causes code like

    val seq = Seq(Double.box(1d), "")
    val elem = seq.head

to infer types that mention `Constable`: `seq` is of type
`Seq[Constable with ...]` and `elem` of type `Constable with ...`.
The latter erases to `Constable`, which results in linking errors.

It is unfortunately impossible to correctly implement `Constable`
in Scala Native, because it requires reflection, so the general
issue is not fixable.

This commit works around the issue in our own test suite, by
ascribing some values with explicit types, so that `Constable` does
not appear in erased types.
WojciechMazur pushed a commit to WojciechMazur/scala-native that referenced this pull request Aug 25, 2021
…-native#1941)

`java.lang.constant.Constable` was introduced at JDK12 and is a
super trait of core boxed classes. This causes code like

    val seq = Seq(Double.box(1d), "")
    val elem = seq.head

to infer types that mention `Constable`: `seq` is of type
`Seq[Constable with ...]` and `elem` of type `Constable with ...`.
The latter erases to `Constable`, which results in linking errors.

It is unfortunately impossible to correctly implement `Constable`
in Scala Native, because it requires reflection, so the general
issue is not fixable.

This commit works around the issue in our own test suite, by
ascribing some values with explicit types, so that `Constable` does
not appear in erased types.
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.

None yet

3 participants