From 707d380b4c6c05772bf9c30ebefedd19df33d224 Mon Sep 17 00:00:00 2001 From: Guillaume Raffin Date: Mon, 20 Jul 2020 22:43:02 +0200 Subject: [PATCH 1/2] Fix #7494: Run bounds checks on java compilation units 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. --- .../src/dotty/tools/dotc/typer/FrontEnd.scala | 9 +++++++ .../dotty/tools/dotc/typer/JavaChecks.scala | 26 +++++++++++++++++++ .../dotty/tools/vulpix/ParallelTesting.scala | 8 +++--- tests/neg/WrongBounds.java | 7 +++++ tests/neg/java-wrong-bounds/D_1.scala | 5 ++++ tests/neg/java-wrong-bounds/J_2.java | 23 ++++++++++++++++ 6 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/typer/JavaChecks.scala create mode 100644 tests/neg/WrongBounds.java create mode 100644 tests/neg/java-wrong-bounds/D_1.scala create mode 100644 tests/neg/java-wrong-bounds/J_2.java diff --git a/compiler/src/dotty/tools/dotc/typer/FrontEnd.scala b/compiler/src/dotty/tools/dotc/typer/FrontEnd.scala index 774cac41cecd..46ddfd654968 100644 --- a/compiler/src/dotty/tools/dotc/typer/FrontEnd.scala +++ b/compiler/src/dotty/tools/dotc/typer/FrontEnd.scala @@ -84,6 +84,13 @@ class FrontEnd extends Phase { case ex: CompilationUnit.SuspendException => } + 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) @@ -113,6 +120,8 @@ class FrontEnd extends Phase { 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 diff --git a/compiler/src/dotty/tools/dotc/typer/JavaChecks.scala b/compiler/src/dotty/tools/dotc/typer/JavaChecks.scala new file mode 100644 index 000000000000..89caf5e1c474 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/typer/JavaChecks.scala @@ -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) +} diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index a09f9de2c5dd..37b9e852e030 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -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) @@ -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") diff --git a/tests/neg/WrongBounds.java b/tests/neg/WrongBounds.java new file mode 100644 index 000000000000..b8db1920cd96 --- /dev/null +++ b/tests/neg/WrongBounds.java @@ -0,0 +1,7 @@ +import java.util.*; + +class WrongBounds { + class LL extends ArrayList> {} + class Wrap>> extends ArrayList {} + class Wrong> extends Wrap {} // error +} diff --git a/tests/neg/java-wrong-bounds/D_1.scala b/tests/neg/java-wrong-bounds/D_1.scala new file mode 100644 index 000000000000..1d09bef9f8a8 --- /dev/null +++ b/tests/neg/java-wrong-bounds/D_1.scala @@ -0,0 +1,5 @@ +class A + +class D[T >: A](v: T) { + def getV(): T = v +} diff --git a/tests/neg/java-wrong-bounds/J_2.java b/tests/neg/java-wrong-bounds/J_2.java new file mode 100644 index 000000000000..d1ee3595bc2c --- /dev/null +++ b/tests/neg/java-wrong-bounds/J_2.java @@ -0,0 +1,23 @@ +public class J_2 extends D { // error + + public J_2() { + super(null); + } + + public static D getDS() { // error + return new D("DS"); + } + + public static final D fieldDS = new D("DS"); // error + + public static void useDS(D ds) {} // error + + public static > void genericDS() {} // error + + public static void useOK(D ds) {} + + public static D getOK() {return null;} + + public static > D genericOK(A a) {return a;} + +} From 2bc92db48bd3e54ceaa2edc0b126e56a52b3f0b9 Mon Sep 17 00:00:00 2001 From: Guillaume Raffin Date: Mon, 20 Jul 2020 22:43:56 +0200 Subject: [PATCH 2/2] Use the new control syntax in Pickler and FrontEnd --- .../src/dotty/tools/dotc/transform/Pickler.scala | 2 +- compiler/src/dotty/tools/dotc/typer/FrontEnd.scala | 13 +++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Pickler.scala b/compiler/src/dotty/tools/dotc/transform/Pickler.scala index 1e82218cae11..b1cc3493a701 100644 --- a/compiler/src/dotty/tools/dotc/transform/Pickler.scala +++ b/compiler/src/dotty/tools/dotc/transform/Pickler.scala @@ -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)) diff --git a/compiler/src/dotty/tools/dotc/typer/FrontEnd.scala b/compiler/src/dotty/tools/dotc/typer/FrontEnd.scala index 46ddfd654968..6d57bb52f0ac 100644 --- a/compiler/src/dotty/tools/dotc/typer/FrontEnd.scala +++ b/compiler/src/dotty/tools/dotc/typer/FrontEnd.scala @@ -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 @@ -91,17 +90,16 @@ class FrontEnd extends Phase { } - private def firstTopLevelDef(trees: List[tpd.Tree])(using Context): Symbol = trees match { + 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}") @@ -113,7 +111,7 @@ 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) @@ -132,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") }