Fix dotr -tasty #3436

nicolasstucki · 2017-11-03T16:30:01Z

No description provided.

smarter · 2017-11-03T22:53:23Z

project/Build.scala

        val scalaLib = attList
          .map(_.data.getAbsolutePath)
          .find(_.contains("scala-library"))
          .toList.mkString(":")

-        if (java == "")
+        if (args.isEmpty) {
+          println("Couldn't run `dotr` without args. Use `repl` to run the repl or add args to run the dotty application")


Why not call the repl task at this point instead?

I tried with repl.evaluated but then dotr only ran repl ignoring the nonempty args. Then I just decided to show this message as we don't really need more in the sbt project.

sbt task evaluation is tricky, you probably need to use a dynamic task to get that to work.

That is what I thought, hence settled for the message. My main concerns were the factorisation of code and checking the it was executing correctly.

I don't think we should special case for empty args. We need a better solution:
If I run dotr -Xprint:typer, args is not empty, but I still want to run the REPL

you can at least run repl -Xprint:typer.

allanrenucci · 2017-11-04T18:19:21Z

project/Build.scala

@@ -640,6 +617,19 @@ object Build {
      }
  )

+  def dotDynTask(main: String) = Def.inputTaskDyn {


I think runCompilerMain is a better name

allanrenucci · 2017-11-04T18:22:54Z

project/Build.scala

          println("Couldn't find java executable on path, please install java to a default location")
-        else if (scalaLib == "") {
+        } else if (scalaLib == "") {
          println("Couldn't find scala-library on classpath, please run using script in bin dir instead")
        } else {
          val dottyLib = packageAll.value("dotty-library")
          s"""$java -classpath .:$dottyLib:$scalaLib ${args.mkString(" ")}""".!


Since you're refactoring. Triple quotes is not needed here

allanrenucci · 2017-11-04T18:29:06Z

.drone.yml

@@ -25,7 +25,7 @@ pipeline:
    image: lampepfl/dotty:2017-10-20
    commands:
      - cp -R . /tmp/1/ && cd /tmp/1/
-      - ./project/scripts/sbt ";compile ;testAll ;dotty-bench/jmh:run 1 1 tests/pos/alias.scala"
+      - ./project/scripts/sbt ";compile ;testAll ;dotty-bench/jmh:run 1 1 tests/pos/alias.scala; ;dotc tests/run/arraycopy.scala ;dotr Test"


tests/pos/alias.scala; ;dotc double semicolon?

Also not sure it is the best place to test an sbt task. @smarter Do you think it should be a scripted test?

Scripted tests are always good in my opinion :)

allanrenucci · 2017-11-04T18:33:30Z

project/Build.scala

        val scalaLib = attList
          .map(_.data.getAbsolutePath)
          .find(_.contains("scala-library"))
          .toList.mkString(":")

-        if (java == "")
+        if (args.isEmpty) {
+          println("Couldn't run `dotr` without args. Use `repl` to run the repl or add args to run the dotty application")


I don't think we should special case for empty args. We need a better solution:
If I run dotr -Xprint:typer, args is not empty, but I still want to run the REPL

nicolasstucki · 2017-11-13T17:17:55Z

~~Next step (in another PR) will be to remove the dotty.tools.dot.FromTasty driver and integrate it to dotty.tools.dot.Main. This will make -tasty an actual ScalaSetings flag.~~
Done :)

smarter · 2017-11-14T22:46:36Z

dist/bin/dotc

@@ -82,7 +81,7 @@ case "$1" in
    # Optimize for short-running applications, see https://github.com/lampepfl/dotty/issues/222
    -Oshort) addJava "-XX:+TieredCompilation -XX:TieredStopAtLevel=1" && shift ;;
        -repl) PROG_NAME="$ReplMain" && shift ;;
-       -tasty) addScala "-Yretain-trees" && PROG_NAME="$FromTasty" && shift ;;
+       -tasty) addScala "-Yretain-trees" && PROG_NAME="$CompilerMain" && shift ;;


I would just make -Yretain-trees default to true when -tasty is used.

I mean, in the compiler itself, instead of doing it in various scripts.

smarter

Otherwise LGTM.

nicolasstucki · 2017-11-15T12:48:31Z

Last commit needs review

OlivierBlanvillain · 2017-11-15T13:44:27Z

project/scripts/sbtBootstrappedTests

+# check that `dotc` compiles and `dotr` runs it
+echo "testing sbt dotc and dotr"
+mkdir out/scriptedtest0
+./dist-bootstrapped/target/pack/bin/dotc tests/pos/sbtDotrTest.scala -d out/scriptedtest0


Any reason to use dist-bootstrapped/**/dotc instead of bin/dotc?

I wanted to make sure I was testing that version of the script. We could also add some tests for bin/dotc in the future.

bin/dotc just forwards to dist-bootstrapped/target/pack/bin/dotc and calls sbt dist-bootstrapped/pack if needed :)

Ok, I'll change it.

Fix #3496: Insert classpath before other arguments

nicolasstucki requested a review from allanrenucci November 3, 2017 16:30

smarter reviewed Nov 3, 2017

View reviewed changes

allanrenucci reviewed Nov 4, 2017

View reviewed changes

nicolasstucki force-pushed the abstract-dotc-repl branch 3 times, most recently from 6cf735e to be81325 Compare November 8, 2017 09:58

nicolasstucki added the stat:needs review label Nov 9, 2017

nicolasstucki requested a review from allanrenucci November 9, 2017 10:13

nicolasstucki assigned allanrenucci Nov 11, 2017

nicolasstucki force-pushed the abstract-dotc-repl branch from be81325 to 53e24a5 Compare November 12, 2017 13:46

nicolasstucki changed the title ~~Abstract dotc repl~~ Fix dotr -tasty Nov 12, 2017

nicolasstucki requested review from smarter and removed request for allanrenucci November 12, 2017 14:23

nicolasstucki assigned smarter and unassigned allanrenucci Nov 12, 2017

nicolasstucki force-pushed the abstract-dotc-repl branch 2 times, most recently from 069c36d to 7c4d1d7 Compare November 12, 2017 16:12

smarter reviewed Nov 14, 2017

View reviewed changes

smarter approved these changes Nov 14, 2017

View reviewed changes

nicolasstucki force-pushed the abstract-dotc-repl branch from d287c70 to 7b9c17f Compare November 15, 2017 07:00

nicolasstucki added 9 commits November 15, 2017 11:28

Add sbt dotr instuctions for repl

b47c487

Factor sbt dotr/repl common init code

ec084ec

Add simple regression tests for sbt dotc/dotr

5aaed79

Improve implementation of sbt dotc

660cb40

Rename sbt dotc/repl abstraction to runCompilerMain

47ae1cf

Move sbt tests to scripted tests

20c72f4

Check sbt dotr output

e133b9e

Fix dotr -tasty

26cdb57

Fix sbt dotr with -classpath

106b15d

nicolasstucki added 3 commits November 15, 2017 11:28

Add regression test for dotc -tasty and dotr -classpath

6889d44

Extract inner classes from FromTasty

178402e

Make -tasty a ScalaSettings flag and remove FromTasty

fdbab48

nicolasstucki force-pushed the abstract-dotc-repl branch from 7b9c17f to a809cfd Compare November 15, 2017 10:54

Do not insert -Yretain-trees when compiling with -tasty

6706b9e

nicolasstucki force-pushed the abstract-dotc-repl branch from a809cfd to 6706b9e Compare November 15, 2017 12:27

OlivierBlanvillain reviewed Nov 15, 2017

View reviewed changes

Change dist-bootstrapped/target/pack/bin to bin

faedad7

nicolasstucki mentioned this pull request Nov 16, 2017

Create dotc -decompiler #3485

Merged

OlivierBlanvillain approved these changes Nov 16, 2017

View reviewed changes

nicolasstucki merged commit fe24786 into scala:master Nov 16, 2017

smarter mentioned this pull request Nov 17, 2017

"sbt dotr" does not automatically put scala-library on the classpath anymore #3496

Closed

nicolasstucki referenced this pull request Nov 17, 2017

Merge pull request #3498 from dotty-staging/fix-#3436

0155804

Fix #3496: Insert classpath before other arguments

allanrenucci deleted the abstract-dotc-repl branch December 14, 2017 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dotr -tasty #3436

Fix dotr -tasty #3436

nicolasstucki commented Nov 3, 2017

smarter Nov 3, 2017

nicolasstucki Nov 4, 2017

smarter Nov 4, 2017

nicolasstucki Nov 4, 2017

allanrenucci Nov 4, 2017

nicolasstucki Nov 6, 2017

allanrenucci Nov 4, 2017

allanrenucci Nov 4, 2017

allanrenucci Nov 4, 2017

allanrenucci Nov 4, 2017

smarter Nov 4, 2017

allanrenucci Nov 4, 2017

nicolasstucki commented Nov 13, 2017 •

edited

smarter Nov 14, 2017

smarter Nov 14, 2017

smarter left a comment

nicolasstucki commented Nov 15, 2017

OlivierBlanvillain Nov 15, 2017

nicolasstucki Nov 15, 2017

OlivierBlanvillain Nov 15, 2017 •

edited

nicolasstucki Nov 15, 2017

Fix dotr -tasty #3436

Fix dotr -tasty #3436

Conversation

nicolasstucki commented Nov 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolasstucki commented Nov 13, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarter left a comment

Choose a reason for hiding this comment

nicolasstucki commented Nov 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OlivierBlanvillain Nov 15, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolasstucki commented Nov 13, 2017 •

edited

OlivierBlanvillain Nov 15, 2017 •

edited