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

The inferred AppliedTypes from Java are not checked #7494

Closed
noti0na1 opened this issue Nov 4, 2019 · 3 comments · Fixed by #9370
Closed

The inferred AppliedTypes from Java are not checked #7494

noti0na1 opened this issue Nov 4, 2019 · 3 comments · Fixed by #9370

Comments

@noti0na1
Copy link
Member

noti0na1 commented Nov 4, 2019

We could construct AppliedTypes from Java whose arguments do not conform to type bound. When the type is used in Dotty, it is not checked.

minimized code

S.scala

class A

class D[T >: A](v: T) {
  def getV(): T = v // ensure T is correctly inferred
}

object S {
  // J.getDS() : D[String] is inferred but not checked
  val dv: String = J.getDS().getV()
}

J.java

public class J {
  // for java, D is D<T extends Object>
  public static D<String> getDS() {
    return new D<String>("DS");
  }
}

expectation

I expected to see a type error at J.getDS() in S.scala, since String does not conform to lower bound A. However, this test passed. I also test the same code in scalac 2.13, and it also compiled.

@TheElectronWill
Copy link
Contributor

TheElectronWill commented Nov 5, 2019

// for java, D is D<T extends Object>

Wait a minute, why isn't it D<T super A>? 🤔
I know Java's generics aren't perfect but I would expect this case to work properly.

@noti0na1
Copy link
Member Author

noti0na1 commented Nov 5, 2019

@TheElectronWill Thanks!

I don't think class D<T super A> is allowed in Java. There is no lower bound allowed in Type Parameters. The lower bound can only be used for widecard in Type Arguments of Parameterized Types, for example: Reference(T referent, ReferenceQueue<? super T> queue).

See: 9.1.2. Generic Interfaces and Type Parameters and 4.5.1. Type Arguments of Parameterized Types .

There are many types in Dotty which cannot be translated in Java, the lower bound is just one case. Another example is OrType: D[T <: A | B].

@TheElectronWill
Copy link
Contributor

You're right! D<T super A> isn't valid Java. I guess using Scala has rubbed off on my Java knowledge 😄

@TheElectronWill TheElectronWill self-assigned this Jul 14, 2020
TheElectronWill added a commit to TheElectronWill/dotty that referenced this issue Jul 21, 2020
Checking every Select of a java-defined symbol would be too costly and still not perfect.
The implemented compromise is to check bounds during the compilation of .java source files.
To do this, a new JavaChecks file is added and called by FrontEnd.
We can't simply discard java compilation units after PostTyper (instead of after Typer),
because sbt.ExtractDependencies (and maybe others) runs before PostTyper and cannot process java sources.
@TheElectronWill TheElectronWill removed their assignment Jul 21, 2020
odersky added a commit to dotty-staging/dotty that referenced this issue Jun 25, 2023
Something goes wrong when we try to do mutually recursive F-bounds checking in Java units.
I am not sure what exactly. But in any case, Scala should not try to do Java's typechecking.
So we now check applications in Java units only if they refer to Scala classes.

I added a test for scala#7494, which was originally addressed by scala#9370, the PR which introduced the Java bounds
checking. Adding the test ensures that the new restrictions still glag the original error.

Fixes scala#17763
smarter added a commit that referenced this issue Jun 26, 2023
Something goes wrong when we try to do mutually recursive F-bounds
checking in Java units. I am not sure what exactly. But in any case,
Scala should not try to do Java's typechecking. So we now check
applications in Java units only if they refer to Scala classes.

I added a test for #7494, which was originally addressed by #9370, the
PR which introduced the Java bounds checking. Adding the test ensures
that the new restrictions still flag the original error.

Fixes #17763
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants