Skip to content

Fix #7494: check type bounds during java joint compilation #9370

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 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Pickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class Pickler extends Phase {

override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] = {
val result = super.runOn(units)
if (ctx.settings.YtestPickler.value)
if ctx.settings.YtestPickler.value
testUnpickler(
using ctx.fresh
.setPeriod(Period(ctx.runId + 1, FirstPhaseId))
Expand Down
22 changes: 14 additions & 8 deletions compiler/src/dotty/tools/dotc/typer/FrontEnd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,10 @@ class FrontEnd extends Phase {

def monitor(doing: String)(body: => Unit)(using Context): Unit =
try body
catch {
catch
case NonFatal(ex) =>
report.echo(s"exception occurred while $doing ${ctx.compilationUnit}")
throw ex
}

def parse(using Context): Unit = monitor("parsing") {
val unit = ctx.compilationUnit
Expand Down Expand Up @@ -84,17 +83,23 @@ class FrontEnd extends Phase {
case ex: CompilationUnit.SuspendException =>
}

private def firstTopLevelDef(trees: List[tpd.Tree])(using Context): Symbol = trees match {
def javaCheck(using Context): Unit = monitor("checking java") {
val unit = ctx.compilationUnit
if unit.isJava then
JavaChecks.check(unit.tpdTree)
}


private def firstTopLevelDef(trees: List[tpd.Tree])(using Context): Symbol = trees match
case PackageDef(_, defs) :: _ => firstTopLevelDef(defs)
case Import(_, _) :: defs => firstTopLevelDef(defs)
case (tree @ TypeDef(_, _)) :: _ => tree.symbol
case _ => NoSymbol
}

protected def discardAfterTyper(unit: CompilationUnit)(using Context): Boolean =
unit.isJava || unit.suspended

override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] = {
override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] =
val unitContexts =
for unit <- units yield
report.inform(s"compiling ${unit.source}")
Expand All @@ -106,13 +111,15 @@ class FrontEnd extends Phase {
enterSyms(using remaining.head)
remaining = remaining.tail

if (firstXmlPos.exists && !defn.ScalaXmlPackageClass.exists)
if firstXmlPos.exists && !defn.ScalaXmlPackageClass.exists then
report.error("""To support XML literals, your project must depend on scala-xml.
|See https://github.com/scala/scala-xml for more information.""".stripMargin,
firstXmlPos)

unitContexts.foreach(typeCheck(using _))
record("total trees after typer", ast.Trees.ntrees)
unitContexts.foreach(javaCheck(using _)) // after typechecking to avoid cycles

val newUnits = unitContexts.map(_.compilationUnit).filterNot(discardAfterTyper)
val suspendedUnits = ctx.run.suspendedUnits
if newUnits.isEmpty && suspendedUnits.nonEmpty && !ctx.reporter.errorsReported then
Expand All @@ -123,14 +130,13 @@ class FrontEnd extends Phase {
| ${suspendedUnits.toList}%, %
|"""
val enableXprintSuspensionHint =
if (ctx.settings.XprintSuspension.value) ""
if ctx.settings.XprintSuspension.value then ""
else "\n\nCompiling with -Xprint-suspension gives more information."
report.error(em"""Cyclic macro dependencies $where
|Compilation stopped since no further progress can be made.
|
|To fix this, place macros in one set of files and their callers in another.$enableXprintSuspensionHint""")
newUnits
}

def run(using Context): Unit = unsupported("run")
}
Expand Down
26 changes: 26 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/JavaChecks.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package dotty.tools.dotc
package typer

import core.Contexts._
import ast.tpd._

/** PostTyper doesn't run on java sources,
* but some checks still need to be applied.
*/
object JavaChecks {
/** Check the bounds of AppliedTypeTrees. */
private object AppliedTypeChecker extends TreeTraverser {
def traverse(tree: Tree)(using Context): Unit = tree match
case tpt: TypeTree =>
Checking.checkAppliedTypesIn(tpt)
case tree: AppliedTypeTree =>
Checking.checkAppliedType(tree)
case _ =>
traverseChildren(tree)
}

/** Scan a tree and check it. */
def check(tree: Tree)(using Context): Unit =
report.debuglog("checking type bounds in " + ctx.compilationUnit.source.name)
AppliedTypeChecker.traverse(tree)
}
8 changes: 5 additions & 3 deletions compiler/test/dotty/tools/vulpix/ParallelTesting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,10 @@ trait ParallelTesting extends RunnerOrchestration { self =>
*/
final def checkFile(testSource: TestSource): Option[JFile] = (testSource match {
case ts: JointCompilationSource =>
ts.files.collectFirst { case f if !f.isDirectory => new JFile(f.getPath.replaceFirst("\\.scala$", ".check")) }

ts.files.collectFirst {
case f if !f.isDirectory =>
new JFile(f.getPath.replaceFirst("\\.(scala|java)$", ".check"))
}
case ts: SeparateCompilationSource =>
Option(new JFile(ts.dir.getPath + ".check"))
}).filter(_.exists)
Expand Down Expand Up @@ -679,7 +681,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
def getErrorMapAndExpectedCount(files: Seq[JFile]): (HashMap[String, Integer], Int) = {
val errorMap = new HashMap[String, Integer]()
var expectedErrors = 0
files.filter(_.getName.endsWith(".scala")).foreach { file =>
files.filter(isSourceFile).foreach { file =>
Using(Source.fromFile(file, "UTF-8")) { source =>
source.getLines.zipWithIndex.foreach { case (line, lineNbr) =>
val errors = line.toSeq.sliding("// error".length).count(_.unwrap == "// error")
Expand Down
7 changes: 7 additions & 0 deletions tests/neg/WrongBounds.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import java.util.*;

class WrongBounds {
class LL<T> extends ArrayList<List<T>> {}
class Wrap<T extends List<List<?>>> extends ArrayList<T> {}
class Wrong<T extends LL<?>> extends Wrap<T> {} // error
}
5 changes: 5 additions & 0 deletions tests/neg/java-wrong-bounds/D_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class A

class D[T >: A](v: T) {
def getV(): T = v
}
23 changes: 23 additions & 0 deletions tests/neg/java-wrong-bounds/J_2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
public class J_2 extends D<String> { // error

public J_2() {
super(null);
}

public static D<String> getDS() { // error
return new D<String>("DS");
}

public static final D<String> fieldDS = new D<String>("DS"); // error

public static void useDS(D<String> ds) {} // error

public static <A extends D<String>> void genericDS() {} // error

public static void useOK(D<?> ds) {}

public static D<?> getOK() {return null;}

public static <A extends D<?>> D<?> genericOK(A a) {return a;}

}