From a433f477961e8971d964c8eb532ebdfa50c9a023 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 6 Dec 2021 12:38:03 +0100 Subject: [PATCH 1/4] fix #13994: initialise inline ctx in lateEnter --- compiler/src/dotty/tools/dotc/Run.scala | 33 +++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index b9552d97fca7..751973065d0a 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -9,7 +9,7 @@ import Types._ import Scopes._ import Names.Name import Denotations.Denotation -import typer.Typer +import typer.{Typer, PrepareInlineable} import typer.ImportInfo._ import Decorators._ import io.{AbstractFile, PlainFile, VirtualFile} @@ -303,17 +303,30 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint .withRootImports def process()(using Context) = { - unit.untpdTree = + + def parseSource()(using Context) = if (unit.isJava) new JavaParser(unit.source).parse() else new Parser(unit.source).parse() - ctx.typer.lateEnter(unit.untpdTree) - def processUnit() = { - unit.tpdTree = ctx.typer.typedExpr(unit.untpdTree) - val phase = new transform.SetRootTree() - phase.run - } - if (typeCheck) - if (compiling) finalizeActions += (() => processUnit()) else processUnit() + + def enterTrees()(using Context) = + ctx.typer.lateEnter(unit.untpdTree) + def typeCheckUnit()(using Context) = + unit.tpdTree = ctx.typer.typedExpr(unit.untpdTree) + val phase = new transform.SetRootTree() + phase.run + if typeCheck then + val typerCtx: Context = + // typer phase allows implicits to be searched + ctx.withPhase(Phases.typerPhase) + if compiling then finalizeActions += (() => typeCheckUnit()(using typerCtx)) + else typeCheckUnit()(using typerCtx) + + unit.untpdTree = parseSource() + val namerCtx = + // inline body annotations are set in namer, capturing the current context + // we need to prepare the context for inlining. + if unit.isJava then ctx else PrepareInlineable.initContext(ctx) + enterTrees()(using namerCtx) } process()(using unitCtx) } From 1f81426e19c66977c1dc9bb97915dbc0e9dec5a3 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 6 Dec 2021 18:40:12 +0100 Subject: [PATCH 2/4] reproduce #13994 in cmdTest --- compiler/src/dotty/tools/dotc/Run.scala | 8 +++--- project/scripts/bootstrapCmdTests | 9 +++++++ project/scripts/cmdTestsCommon.inc.sh | 4 ++- project/scripts/sbt | 15 +++++++++-- tests/cmdTest-sbt-tests/README.md | 7 +++++ .../sourcepath-with-inline/build.sbt | 17 ++++++++++++ .../changes/zz.new.scala | 7 +++++ .../changes/zz.original.scala | 6 +++++ .../project/DottyInjectedPlugin.scala | 27 +++++++++++++++++++ .../project/build.properties | 1 + .../src/main/scala/a/Bar.scala | 5 ++++ 11 files changed, 98 insertions(+), 8 deletions(-) create mode 100644 tests/cmdTest-sbt-tests/README.md create mode 100644 tests/cmdTest-sbt-tests/sourcepath-with-inline/build.sbt create mode 100644 tests/cmdTest-sbt-tests/sourcepath-with-inline/changes/zz.new.scala create mode 100644 tests/cmdTest-sbt-tests/sourcepath-with-inline/changes/zz.original.scala create mode 100644 tests/cmdTest-sbt-tests/sourcepath-with-inline/project/DottyInjectedPlugin.scala create mode 100644 tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties create mode 100644 tests/cmdTest-sbt-tests/sourcepath-with-inline/src/main/scala/a/Bar.scala diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index 751973065d0a..e439ebfd60f7 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -304,10 +304,6 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint def process()(using Context) = { - def parseSource()(using Context) = - if (unit.isJava) new JavaParser(unit.source).parse() - else new Parser(unit.source).parse() - def enterTrees()(using Context) = ctx.typer.lateEnter(unit.untpdTree) def typeCheckUnit()(using Context) = @@ -321,7 +317,9 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint if compiling then finalizeActions += (() => typeCheckUnit()(using typerCtx)) else typeCheckUnit()(using typerCtx) - unit.untpdTree = parseSource() + unit.untpdTree = + if (unit.isJava) new JavaParser(unit.source).parse() + else new Parser(unit.source).parse() val namerCtx = // inline body annotations are set in namer, capturing the current context // we need to prepare the context for inlining. diff --git a/project/scripts/bootstrapCmdTests b/project/scripts/bootstrapCmdTests index 9f8956e1d23c..cc28c5d4e022 100755 --- a/project/scripts/bootstrapCmdTests +++ b/project/scripts/bootstrapCmdTests @@ -83,3 +83,12 @@ clear_out "$OUT" ./bin/scalac -d "$OUT/out.jar" tests/pos/i12973.scala echo "Bug12973().check" | TERM=dumb ./bin/scala -cp "$OUT/out.jar" > "$tmp" 2>&1 grep -qe "Bug12973 is fixed" "$tmp" + +echo "testing -sourcepath with inlining" +cwd=$(pwd) +java_prop="-Dpack.version.file=$cwd/dist/target/pack/VERSION" +sbt_test_command=";clean;prepareSources;compile;copyChanges;compile" +(cd "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline" && "$SBT" "$java_prop" "$sbt_test_command") +rm -rf "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline/target" +rm -rf "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/target" +rm -f "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline/src/main/scala/a/zz.scala" diff --git a/project/scripts/cmdTestsCommon.inc.sh b/project/scripts/cmdTestsCommon.inc.sh index dc81a3e35b52..587dd3527bd2 100644 --- a/project/scripts/cmdTestsCommon.inc.sh +++ b/project/scripts/cmdTestsCommon.inc.sh @@ -1,6 +1,8 @@ set -eux -SBT="./project/scripts/sbt" # if run on CI +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" >& /dev/null && pwd)/../.." + +SBT="$ROOT/project/scripts/sbt" # if run on CI # SBT="sbt" # if run locally SOURCE="tests/pos/HelloWorld.scala" diff --git a/project/scripts/sbt b/project/scripts/sbt index 8c6f5aa1c280..9184056dd3b8 100755 --- a/project/scripts/sbt +++ b/project/scripts/sbt @@ -2,12 +2,23 @@ set -e # Usage: -# ./sbt +# ./sbt -CMD="${1:?Missing sbt command}" +declare -a sys_props +declare -a sbt_commands + +while [[ $# -gt 0 ]]; do + case "$1" in + -D*) sys_props+=("$1") && shift ;; + *) sbt_commands+=("$1") && shift ;; + esac +done + +CMD="${sbt_commands[0]:?Missing sbt command}" # run sbt with the supplied arg sbt -J-XX:ReservedCodeCacheSize=512m \ -DSBT_PGP_USE_GPG=false \ + "${sys_props[@]}" \ -no-colors \ "$CMD" diff --git a/tests/cmdTest-sbt-tests/README.md b/tests/cmdTest-sbt-tests/README.md new file mode 100644 index 000000000000..3738c0861fb1 --- /dev/null +++ b/tests/cmdTest-sbt-tests/README.md @@ -0,0 +1,7 @@ +# Readme + +Do not use this directory for testing sbt projects in general, add a test case to `dotty/sbt-test` + +This directory is for sbt tests that can not be reproduced with sbt scripted tests. + +Adding a test here will reduce the performance of running all tests. diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline/build.sbt b/tests/cmdTest-sbt-tests/sourcepath-with-inline/build.sbt new file mode 100644 index 000000000000..4bff160ff55a --- /dev/null +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline/build.sbt @@ -0,0 +1,17 @@ +import java.util.Properties + +val prepareSources = taskKey[Unit]("Copy changes to the src directory") +val copyChanges = taskKey[Unit]("Copy changes to the src directory") + +val srcDir = settingKey[File]("The directory to copy changes to") +val changesDir = settingKey[File]("The directory to copy changes from") + +srcDir := (ThisBuild / baseDirectory).value / "src" / "main" / "scala" +changesDir := (ThisBuild / baseDirectory).value / "changes" + +prepareSources := IO.copyFile(changesDir.value / "zz.original.scala", srcDir.value / "a" / "zz.scala") +copyChanges := IO.copyFile(changesDir.value / "zz.new.scala", srcDir.value / "a" / "zz.scala") + +(Compile / scalacOptions) ++= Seq( + "-sourcepath", (Compile / sourceDirectories).value.map(_.getAbsolutePath).distinct.mkString(java.io.File.pathSeparator), +) diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline/changes/zz.new.scala b/tests/cmdTest-sbt-tests/sourcepath-with-inline/changes/zz.new.scala new file mode 100644 index 000000000000..fbf5cf7fb5e0 --- /dev/null +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline/changes/zz.new.scala @@ -0,0 +1,7 @@ +package a + +object Foo: // note that `Foo` is defined in `zz.scala` + class Local + inline def foo(using Local): Nothing = + ??? + ??? diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline/changes/zz.original.scala b/tests/cmdTest-sbt-tests/sourcepath-with-inline/changes/zz.original.scala new file mode 100644 index 000000000000..17a7488ccb1a --- /dev/null +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline/changes/zz.original.scala @@ -0,0 +1,6 @@ +package a + +object Foo: // note that `Foo` is defined in `zz.scala` + class Local + inline def foo(using Local): Nothing = + ??? diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/DottyInjectedPlugin.scala b/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/DottyInjectedPlugin.scala new file mode 100644 index 000000000000..1cdccf273e9a --- /dev/null +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/DottyInjectedPlugin.scala @@ -0,0 +1,27 @@ +import sbt._ +import Keys._ + +import java.util.Properties +import java.io.File + +object DottyInjectedPlugin extends AutoPlugin { + override def requires = plugins.JvmPlugin + override def trigger = allRequirements + + private val packProperties = settingKey[Properties]("The properties of the dist/pack command") + + override val projectSettings = Seq( + packProperties := { + val prop = new Properties() + IO + .read(new File(sys.props("pack.version.file"))) + .linesIterator + .foreach { line => + val Array(key, value) = line.split(":=") + prop.setProperty(key, value) + } + prop + }, + scalaVersion := packProperties.value.getProperty("version") + ) +} diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties b/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties new file mode 100644 index 000000000000..10fd9eee04ac --- /dev/null +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties @@ -0,0 +1 @@ +sbt.version=1.5.5 diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline/src/main/scala/a/Bar.scala b/tests/cmdTest-sbt-tests/sourcepath-with-inline/src/main/scala/a/Bar.scala new file mode 100644 index 000000000000..79af4eb2cebd --- /dev/null +++ b/tests/cmdTest-sbt-tests/sourcepath-with-inline/src/main/scala/a/Bar.scala @@ -0,0 +1,5 @@ +package a + +object Bar: + given Foo.Local() + def Bar = Foo.foo From 573b083a77e3a7d964fae9e702c6dde94dae5fbb Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 13 Dec 2021 13:19:53 +0100 Subject: [PATCH 3/4] move lateCompile implementation to Namer --- compiler/src/dotty/tools/dotc/Run.scala | 33 ++++--------- .../src/dotty/tools/dotc/typer/Namer.scala | 47 +++++++++++++++---- 2 files changed, 47 insertions(+), 33 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index e439ebfd60f7..504cffbe692a 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -23,8 +23,6 @@ import rewrites.Rewrites import profile.Profiler import printing.XprintMode -import parsing.Parsers.Parser -import parsing.JavaParsers.JavaParser import typer.ImplicitRunInfo import config.Feature import StdNames.nme @@ -302,31 +300,16 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint .setCompilationUnit(unit) .withRootImports - def process()(using Context) = { - - def enterTrees()(using Context) = - ctx.typer.lateEnter(unit.untpdTree) - def typeCheckUnit()(using Context) = - unit.tpdTree = ctx.typer.typedExpr(unit.untpdTree) - val phase = new transform.SetRootTree() - phase.run + def process()(using Context) = + ctx.typer.lateEnterUnit(doTypeCheck => if typeCheck then - val typerCtx: Context = - // typer phase allows implicits to be searched - ctx.withPhase(Phases.typerPhase) - if compiling then finalizeActions += (() => typeCheckUnit()(using typerCtx)) - else typeCheckUnit()(using typerCtx) - - unit.untpdTree = - if (unit.isJava) new JavaParser(unit.source).parse() - else new Parser(unit.source).parse() - val namerCtx = - // inline body annotations are set in namer, capturing the current context - // we need to prepare the context for inlining. - if unit.isJava then ctx else PrepareInlineable.initContext(ctx) - enterTrees()(using namerCtx) + if compiling then finalizeActions += doTypeCheck + else doTypeCheck() + ) + + inContext(unitCtx) { + process() } - process()(using unitCtx) } private sealed trait PrintedTree diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 9398b50db5e5..7d428e0fe1c4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -18,6 +18,8 @@ import tpd.tpes import Variances.alwaysInvariant import config.{Config, Feature} import config.Printers.typr +import parsing.JavaParsers.JavaParser +import parsing.Parsers.Parser import Annotations._ import Inferencing._ import transform.ValueClasses._ @@ -708,15 +710,44 @@ class Namer { typer: Typer => ctxWithStats } - /** Index symbols in `tree` while asserting the `lateCompile` flag. - * This will cause any old top-level symbol with the same fully qualified - * name as a newly created symbol to be replaced. + /** Parse the source and index symbols in the compilation unit's untpdTree + * while asserting the `lateCompile` flag. This will cause any old + * top-level symbol with the same fully qualified name as a newly created + * symbol to be replaced. + * + * Will call the callback with an implementation of type checking + * That will set the tpdTree and root tree for the compilation unit. */ - def lateEnter(tree: Tree)(using Context): Context = { - val saved = lateCompile - lateCompile = true - try index(tree :: Nil) finally lateCompile = saved - } + def lateEnterUnit(typeCheckCB: (() => Unit) => Unit)(using Context) = + val unit = ctx.compilationUnit + + /** Index symbols in unit.untpdTree with lateCompile flag = true */ + def lateEnter()(using Context): Context = + val saved = lateCompile + lateCompile = true + try index(unit.untpdTree :: Nil) finally lateCompile = saved + + /** Set the tpdTree and root tree of the compilation unit */ + def lateTypeCheck()(using Context) = + unit.tpdTree = typer.typedExpr(unit.untpdTree) + val phase = new transform.SetRootTree() + phase.run + + unit.untpdTree = + if (unit.isJava) new JavaParser(unit.source).parse() + else new Parser(unit.source).parse() + + atPhase(Phases.typerPhase) { + inContext(PrepareInlineable.initContext(ctx)) { + // inline body annotations are set in namer, capturing the current context + // we need to prepare the context for inlining. + lateEnter() + typeCheckCB { () => + lateTypeCheck() + } + } + } + end lateEnterUnit /** The type bound on wildcard imports of an import list, with special values * Nothing if no wildcard imports of this kind exist From 0d7b6450e8f0763a5bcf07592f6580502f4c3f1f Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 13 Dec 2021 16:47:03 +0100 Subject: [PATCH 4/4] don't load scala version from java properties --- compiler/src/dotty/tools/dotc/Run.scala | 4 +-- project/scripts/bootstrapCmdTests | 17 +++++++++--- project/scripts/cmdTestsCommon.inc.sh | 5 ++++ project/scripts/sbt | 15 ++--------- .../project/DottyInjectedPlugin.scala | 27 ------------------- 5 files changed, 22 insertions(+), 46 deletions(-) delete mode 100644 tests/cmdTest-sbt-tests/sourcepath-with-inline/project/DottyInjectedPlugin.scala diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index 504cffbe692a..5c75f0ed8e2c 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -307,9 +307,7 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint else doTypeCheck() ) - inContext(unitCtx) { - process() - } + process()(using unitCtx) } private sealed trait PrintedTree diff --git a/project/scripts/bootstrapCmdTests b/project/scripts/bootstrapCmdTests index cc28c5d4e022..4e5db2a5f3d6 100755 --- a/project/scripts/bootstrapCmdTests +++ b/project/scripts/bootstrapCmdTests @@ -84,11 +84,22 @@ clear_out "$OUT" echo "Bug12973().check" | TERM=dumb ./bin/scala -cp "$OUT/out.jar" > "$tmp" 2>&1 grep -qe "Bug12973 is fixed" "$tmp" +echo "capturing scala version from dist/target/pack/VERSION" +cwd=$(pwd) +IFS=':=' read -ra versionProps < "$cwd/dist/target/pack/VERSION" # temporarily set IFS to ':=' to split versionProps +[ ${#versionProps[@]} -eq 3 ] && \ + [ ${versionProps[0]} = "version" ] && \ + [ -n ${versionProps[2]} ] || die "Expected non-empty 'version' property in $cwd/dist/target/pack/VERSION" +scala_version=${versionProps[2]} + echo "testing -sourcepath with inlining" +# Here we will test that an inline method symbol loaded from the sourcepath (-sourcepath compiler option) +# will have its `defTree` correctly set when its method body is required for inlining. +# So far I have not found a way to replicate issue https://github.com/lampepfl/dotty/issues/13994 +# with sbt scripted tests, if a way is found, move this test there. cwd=$(pwd) -java_prop="-Dpack.version.file=$cwd/dist/target/pack/VERSION" -sbt_test_command=";clean;prepareSources;compile;copyChanges;compile" -(cd "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline" && "$SBT" "$java_prop" "$sbt_test_command") +sbt_test_command="++${scala_version}!;clean;prepareSources;compile;copyChanges;compile" +(cd "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline" && "$SBT" "$sbt_test_command") rm -rf "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline/target" rm -rf "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/target" rm -f "$cwd/tests/cmdTest-sbt-tests/sourcepath-with-inline/src/main/scala/a/zz.scala" diff --git a/project/scripts/cmdTestsCommon.inc.sh b/project/scripts/cmdTestsCommon.inc.sh index 587dd3527bd2..a37ab757c057 100644 --- a/project/scripts/cmdTestsCommon.inc.sh +++ b/project/scripts/cmdTestsCommon.inc.sh @@ -14,6 +14,11 @@ OUT=$(mktemp -d) OUT1=$(mktemp -d) tmp=$(mktemp) +die () { + echo >&2 "$@" + exit 1 +} + clear_out() { local out="$1" diff --git a/project/scripts/sbt b/project/scripts/sbt index 9184056dd3b8..8c6f5aa1c280 100755 --- a/project/scripts/sbt +++ b/project/scripts/sbt @@ -2,23 +2,12 @@ set -e # Usage: -# ./sbt +# ./sbt -declare -a sys_props -declare -a sbt_commands - -while [[ $# -gt 0 ]]; do - case "$1" in - -D*) sys_props+=("$1") && shift ;; - *) sbt_commands+=("$1") && shift ;; - esac -done - -CMD="${sbt_commands[0]:?Missing sbt command}" +CMD="${1:?Missing sbt command}" # run sbt with the supplied arg sbt -J-XX:ReservedCodeCacheSize=512m \ -DSBT_PGP_USE_GPG=false \ - "${sys_props[@]}" \ -no-colors \ "$CMD" diff --git a/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/DottyInjectedPlugin.scala b/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/DottyInjectedPlugin.scala deleted file mode 100644 index 1cdccf273e9a..000000000000 --- a/tests/cmdTest-sbt-tests/sourcepath-with-inline/project/DottyInjectedPlugin.scala +++ /dev/null @@ -1,27 +0,0 @@ -import sbt._ -import Keys._ - -import java.util.Properties -import java.io.File - -object DottyInjectedPlugin extends AutoPlugin { - override def requires = plugins.JvmPlugin - override def trigger = allRequirements - - private val packProperties = settingKey[Properties]("The properties of the dist/pack command") - - override val projectSettings = Seq( - packProperties := { - val prop = new Properties() - IO - .read(new File(sys.props("pack.version.file"))) - .linesIterator - .foreach { line => - val Array(key, value) = line.split(":=") - prop.setProperty(key, value) - } - prop - }, - scalaVersion := packProperties.value.getProperty("version") - ) -}