Skip to content

Include a test for selective dotty bootstrap #1274

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

Closed
odersky opened this issue May 23, 2016 · 7 comments
Closed

Include a test for selective dotty bootstrap #1274

odersky opened this issue May 23, 2016 · 7 comments
Assignees

Comments

@odersky
Copy link
Contributor

odersky commented May 23, 2016

I have been running the script below locally to verify that bootstrap using Tasty works. It first compiles everything, then compiles each subdirectory individually, using Tasty to access info from other directories. Getting this to work took a while, so it would be good to have a test that verifies we can still do this.

Can someone with more test-foo than I give it a stab? Here's the (tcsh) script I was using (run it from dotty/bin):

# To run this script, make sure /classes is on the classpath!
rm -rf /classes/dotty
alias dc      'echo \!! ; java -Xms1g -Xmx3g dotty.tools.dotc.Main -d /classes \!*'
cd ../src/dotty/tools
pwd
echo "compile self"
dc *.scala */*.scala */*/*.scala */*/*/*.scala */*/*/*/*.scala */*/*/*/*/*.scala
dc *.scala
dc dotc/*.scala
dc dotc/ast/*.scala
dc dotc/config/*.scala 
dc dotc/parsing/*.scala
dc dotc/printing/*.scala
dc dotc/repl/*.scala
dc dotc/reporting/*.scala
dc dotc/rewrite/*.scala
dc dotc/transform/*.scala
dc dotc/typer/*.scala
dc dotc/util/*.scala
@odersky odersky changed the title include a test for selective dotty bootstrap Include a test for selective dotty bootstrap May 23, 2016
@odersky odersky mentioned this issue May 23, 2016
@liufengyun liufengyun self-assigned this Jul 28, 2016
@liufengyun
Copy link
Contributor

liufengyun commented Jul 28, 2016

I'm looking at this issue, just had a discussion with @DarkDimius and he cleared my confusion about the classpath for the compiler and the program. I jot down a note here to make sure I understand correctly.

Conceptually, the compiler and program do not share classpath for symbol resolution:

compiler  -->   lib1; lib2;

program   -->   jar1; jar2;

In the program, by default it should NOT be able to use lib1; lib2, nor anything in the compiler. This is exactly the case if we invoke Dotty with -Dscala.usejavacp=false.

However, with -Dscala.usejavacp=true, the program is able to use anything that's in the compiler runtime classpath. That's why there's no need to pass the -classpath option to Dotty when bootstrapping the compiler.

For this task:

  • if we run the test as part of JUnit test(scala.usejavacp=true), we'll need to pass -classpath option to overwrite the class files inside dotty.jar from the compiler runtime.
  • if we run the test as a script with scala.usejavacp=false, then we'll need to pass all dependent libraries to java -classpath as well scala -classpath, and usedotty.jar in the java classpath, while use the unpackaged compile target directory path in scala classpath.

@liufengyun
Copy link
Contributor

liufengyun commented Jul 29, 2016

I just checked settings for running JUnit tests, e.g. dotc_transform. It's a surprise to me that even thoughPathResolver.Environment.useJavaClassPath is false, the compiler is still able to resolve symbols related to stdlib and dotty core(without passing them via scala -classpath).

However, when I run the compiler from command line without -usejavacp and without passing dotty.jar to dotty -classpath, it throws exception when compiling code snippets (as expected):

java -Xmx768m -Xms768m -classpath $classpath dotty.tools.dotc.Main -classpath $SCALA_LIB:$SCALA_REFLECT $@

I'm investigating further.

@liufengyun
Copy link
Contributor

I figured it out by printing ctx.platform.classPath(ctx).packages) and run with different configs.

The unexpected behaviuor is due to the fact that:

  1. By default the jvm option -bootclasspath for running compiler will be available for compiling user programs
  2. When -usejavacp is enabled, the jvm option -classpath for running compiler will be available for compiling user programs.
  3. When we start the compiler using sbt, it's configured in Build.scala that all dependencies as well as the dotty.jar will be part of the jvm -Xbootclasspath option, thus they are all available for compiling the user program. The same holds for bin/dotc, which also adds all jars to the jvm option-bootclasspath.

I believe this design is due to various engineering considerations, and I think it's OK to keep it as it is.

However, to address potential side effects of the above approach (user want to use a different version of default lib) as well as easily fix this issue, I propose a simple change in class resolution:

  • In class resolution, make sure paths passed via -classpath to Dotty take precedence over paths from compiler runtime -bootclasspath and -classpath(when usejavacp enabled).

With this change, we only need to add a JUnit test with the option -classpath ./out/dotty/ to close this issue.

What do you think of this change, @odersky @DarkDimius ?

@DarkDimius
Copy link
Contributor

Junit test are run in forked jvm with environment constructed by hand in
Build.scala. Current build.scala unconditionally puts dotty and all
dependencies on the boot class path.

On 29 July 2016 11:57:11 liu fengyun [email protected] wrote:

I just checked settings for running JUnit tests, e.g. dotc_transform.
It's a surprise to me that even
thoughPathResolver.Environment.useJavaClassPath is false, the compiler
is still able to resolve symbols related to stdlib and dotty core(without
passing them via scala -classpath).

However, when I run the compiler from command line without -usejavacp and
without passing dotty.jar to dotty -classpath, it throws exception when
compiling code snippets (as expected):

java -Xmx768m -Xms768m -classpath $classpath dotty.tools.dotc.Main 
-classpath $SCALA_LIB:$SCALA_REFLECT $@

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1274 (comment)

@DarkDimius
Copy link
Contributor

In class resolution, make sure paths passed via -classpath to Dotty take precedence over paths from compiler runtime -bootclasspath and -classpath(when usejavacp enabled).

With this change, we only need to add a JUnit test with the option -classpath ./out/dotty/ to close this issue.

This would be a nice fist step. But be aware there still may be bugs that would be either hidden or triggered by the fact that multiple versions of the same class are present on the ClassPath. In particular there may be fun interactions with name-mangling of inner classes.

I'd propose to have a custom option, that prohibits loading Scala2 compiled classes that aren't in the whitelist. The whitelist could for example only include classes inside scala.*. This would enforce that when compiling dotty we do not use Scala2 compiled version.

@liufengyun
Copy link
Contributor

liufengyun commented Jul 29, 2016

Thanks a lot for your comments, @DarkDimius . I just pushed a commit to make the simplest change and see how it works out.

Note that previously we already have duplicate class files in the path when running sbt test: (1) from the dotty.jar (2) from target/scala-2.11/classes/.

@smarter
Copy link
Member

smarter commented Jul 29, 2016

See also #1301 and #1300

liufengyun added a commit to dotty-staging/dotty that referenced this issue Aug 2, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Aug 2, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Aug 2, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Aug 8, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Aug 10, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Aug 25, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Oct 12, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Oct 12, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Oct 13, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Oct 13, 2016
liufengyun added a commit to dotty-staging/dotty that referenced this issue Oct 13, 2016
felixmulder added a commit that referenced this issue Oct 13, 2016
fix #1274: test for dotty bootstrap based on tasty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants