Merge pull request #14050 from dotty-staging/fix-13994-initial

fix #13994: initialise inline ctx in lateEnter

Author: odersky

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}
@@ -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,19 +300,13 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
         .setCompilationUnit(unit)
         .withRootImports
 
-      def process()(using Context) = {
-        unit.untpdTree =
-          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 process()(using Context) =
+        ctx.typer.lateEnterUnit(doTypeCheck =>
+          if typeCheck then
+            if compiling then finalizeActions += doTypeCheck
+            else doTypeCheck()
+        )
+
       process()(using unitCtx)
     }
 

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

project/scripts/bootstrapCmdTests

@@ -87,3 +87,23 @@ 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 "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)
+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"

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"
@@ -12,6 +14,11 @@ OUT=$(mktemp -d)
 OUT1=$(mktemp -d)
 tmp=$(mktemp)
 
+die () {
+    echo >&2 "$@"
+    exit 1
+}
+
 clear_out()
 {
   local out="$1"

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.

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),
+)

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 =
+    ???
+    ???

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 =
+    ???

tests/cmdTest-sbt-tests/sourcepath-with-inline/project/build.properties

@@ -0,0 +1 @@
+sbt.version=1.5.5

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