Rewriting tool should warn on extending DelayedInit

[DarkDimius]

Out of 1500 run tests that fail, 900 fail due to them extending scala.App, which relies on DelayedInit, that is not supported.

[DarkDimius]

ping @samuelgruetter

[densh]

@odersky is there a plan to completely remove DelayedInit in the future?

[odersky]

Yes, we should remove delayedInit.

But given the 900 test failures I think it would be reasonable if the
rewriting tool rewrote
"extends App" instead of just issuing a warning. I.e.

object Foo extends App { ... }

becomes

object Foo extends App { override def main(args: Array[String]): Unit =

{ ... }}

If we have macro annotations, we could envision having syntactic sugar like

@app object Foo { ... }

for this. App can be left as a convenience class in dotty, but it would not
have a delayedInit mechanism. More or less all it would do is make the
command line arguments available.

• Martin

On Wed, May 13, 2015 at 1:16 PM, Denys Shabalin [email protected]
wrote:

@odersky https://github.com/odersky is there a plan to completely
remove DelayedInit in the future?

—
Reply to this email directly or view it on GitHub
#559 (comment).

Martin Odersky
EPFL

[DarkDimius]

@odersky For tests, I've added an superclass that has a main method: dotty-staging@28bb276 dotty-staging@28bb276

This is almost equivalent to moving statements into the body of class, the only difference is that tests are actually now executed before main method is called.
This made several hundred tests pass.

[densh]

@app object Foo { ... } looks like a really cool replacement.

[DarkDimius]

Honestly, I dislike the app being so short. In most cases, there are have no-more than two main methods in project. I'd prefer to be explicit here, and write the full @application, as it is easier to read.

[He-Pin]

I like @application too.

[odersky]

@Application / @app, either is fine with me.

On Wed, May 13, 2015 at 2:32 PM, kerr [email protected] wrote:

I like @Application too.

—
Reply to this email directly or view it on GitHub
#559 (comment).

Martin Odersky
EPFL

[densh]

Lets create wiki page that lists all of the expected incompatibilities starting with this one. (Unfortunately I don't have the permission to do this.)

[samuelgruetter]

@densh the list of incompatibilities is currently maintained as a bullet-point list in the readme of the rewrite tool.

So Dmitry's LegacyApp makes the run tests work, but in the future, how should the rewrite tool handle App and DelayedInit?
If App is left as a convenience class in dotty, as Martin suggested, no rewriting will be done for App. So the rewrite tool would only have to warn if DelayedInit is used. But on the other hand, this will also be reported once you try to compile the code with dotty. So this leads to the question if the rewrite tool should warn about library incompatibilities at all, or just focus on the rewriting?

[DarkDimius]

If App is left as a convenience class in dotty, as Martin suggested, no rewriting will be done for App.

App class itself relies on DelayedInit functionality.
While I was able to make run tests work with LegacyApp, this is because none of tests inspects command line arguments.
Other App classes could actually inspect arguments, and I see no non-magical way which can simulate old behavior.

[retronym]

Bikeshedding a little::

object Test1 {
  def main = {
  }
}

And have scala add a main-method bridge. If you want the arguments, write the full main method. Or allow the slightly less jarring signature (as in Java);

object Test2 {
  def main(args: String*) = {
  }
}

I prefer this over @app to avoid perpetuating confusion when people write:

@app object Test {
  val x = 42
}

object Test2 {
   def main(a: Array[String]) = {
     Test.x // null under Scala 2(with a warning). Maybe this wouldn't compile under the macro annotation version?
   } 
}

[retronym]

While I've got the paint out, here's another colour:

@app class C { println("foo") }
@app class D(args: String*) { println(args.length) }

class C { println("foo"); <static> def main(args: Array[String]) = new C() }
class D(args: String*) { println(args.length); <static> def main(args: Array[String]) = new C(WrappedArray(args))  }

[odersky]

On Thu, May 14, 2015 at 1:52 AM, Jason Zaugg [email protected]
wrote:

Bikeshedding a little::

object Test {
def main = {
}
}

And have scala adds a main-method bridge. If you want the arguments, write
the full main method.

I prefer this over:

@app object Test {
val x = 1
}

Because otherwise you create a lot of newbie confusion when they try:

@app object Test {
val x = 42
}
object Test2 {
def main(a: Array[String]) = {
Test.x // null under Scala 2(with a warning). Maybe this wouldn't compile under the macro annotation version?
}
}

My assumption was that it should not complile.

Cheers

• Martin

—

Reply to this email directly or view it on GitHub
#559 (comment).

Martin Odersky
EPFL

[densh]

Maybe we can allow top-level methods in some form (e.g. automatically wrap them into objects internally). This way it won't be confusing and will still be low-overhead syntax-wise.

[odersky]

But I guess the natural encoding of a top-level method

 def f(x: T) = e

would be

object f { def apply(x: T) = e }

So we still would need a way to tell that we want a main, not an apply.

• Martin

On Thu, May 14, 2015 at 10:50 AM, Denys Shabalin [email protected]
wrote:

Maybe we can allow top-level methods in some form (e.g. automatically wrap
them into objects internally). This way it won't be confusing and will
still be low-overhead syntax-wise.

—
Reply to this email directly or view it on GitHub
#559 (comment).

Martin Odersky
EPFL

[densh]

Maybe top-level methods named main will also have a second forwarder method called main generated for them automatically. AFAIK we generate forwarders for main without arguments, this should be quite similar.

[xeno-by]

Sort of an oftopic, but I'd like to say that the encoding for top-level methods sounds really nice. @odersky, would you be interested in a language enhancement proposal akin to #499?

[som-snytt]

Sorry to see App go away (as a restricted usage of DelayedInit). It's handy because it moves code out of the static class initializer, which was a headache for performance-minded REPL users.

scala/scala#4349

[odersky]

The hope is that we will be able to use some kind of annotation macros to
write something like

@app object Foo { ... }

We don't have that yet defined though.

• Martin

On Sat, Jun 6, 2015 at 10:34 PM, som-snytt [email protected] wrote:

Sorry to see App go away (as a restricted usage of DelayedInit). It's
handy because it moves code out of the static class initializer, which was
a headache for performance-minded REPL users.

scala/scala#4349 scala/scala#4349

—
Reply to this email directly or view it on GitHub
#559 (comment).

Martin Odersky
EPFL

[olafurpg]

Scalafix now has a NoExtendsApp rewrite that inserts an explicit main function for objects that extend App. https://scalacenter.github.io/scalafix/#NoExtendsApp

Should there be a rewrite for code that uses DelayedInit directly?

[sjrd]

@olafurpg Does your rewrite deal with things like

object Main extends App {
  println("Starting the app")
  val foo = { println("Initializing foo"); "hello" }
  init()

  def init(): Unit = println(foo)
}

?

[olafurpg]

@sjrd That gets rewritten into

object Main {
  def main(args: Array[String]): Unit = {
    println("Starting the app")
    val foo = { println("Initializing foo"); "hello" }
    init()

    def init(): Unit = println(foo)
  }
}

which prints the same output as before

[info] Running test.Main
Starting the app
Initializing foo
hello

What would you expect it to rewrite into?

[sjrd]

Ah I see. The current DelayedInit rewrite inside the compiler gives:

  object Main extends Object with App {
    ...
    final <stable> <accessor> def initCode(): scala.collection.mutable.ListBuffer = Main.this.initCode;
    private[this] val initCode: scala.collection.mutable.ListBuffer = _;
    @deprecated("The delayedInit mechanism will disappear.", "2.11.0") override def delayedInit(body: Function0): Unit = scala.App$class.delayedInit(Main.this, body);
    @deprecatedOverriding("main should not be overridden", "2.11.0") def main(args: Array[String]): Unit = scala.App$class.main(Main.this, args);
    private[this] val foo: String = _;
    <stable> <accessor> def foo(): String = Main.this.foo;
    def init(): Unit = scala.this.Predef.println(Main.this.foo());
    final <synthetic> def delayedEndpoint$Main$1: Unit = {
      scala.this.Predef.println("Starting the app");
      Main.this.foo = {
        scala.this.Predef.println("Initializing foo");
        "hello"
      };
      Main.this.init();
      ()
    };
    def <init>(): Main.type = {
      Main.super.<init>();
      scala.App$class./*App$class*/$init$(Main.this);
      Main.this.delayedInit(new Main$delayedInit$body(Main.this));
      ()
    }
  };
  final <synthetic> class Main$delayedInit$body extends runtime.AbstractFunction0 {
    ...
  }

where foo is still a field of the object, but it is initialized inside delayedEndpoint$Main$1. Something that cannot be written in surface syntax.

[sjrd]

Your rewrite won't be able to support cases where there are methods in the object that override/implement methods from the superclass.

[olafurpg]

Good catch @sjrd I opened scalacenter/scalafix#215, let's move the discussion there. It should be possible to keep overriden members in the object's template body, although care needs to be taken with eagerly evaluated vals/vars.