Skip to content
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

DelayedInit fails for top derived class has empty constructor #4680

Closed
scabug opened this issue Jun 7, 2011 · 33 comments
Closed

DelayedInit fails for top derived class has empty constructor #4680

scabug opened this issue Jun 7, 2011 · 33 comments

Comments

@scabug
Copy link

@scabug scabug commented Jun 7, 2011

Consider the code:

trait A extends DelayedInit {
  println("A ConstructionCode")

  def delayedInit(body: => Unit) = {
    body
    postConstructionCode
  }
  protected def postConstructionCode: Unit = {
    println("A PostConstructionCode")
  }
}
trait B extends A {
  println("B ConstructionCode")
  override protected def postConstructionCode: Unit = {
    super.postConstructionCode
    println("B PostConstructionCode")
  }
}

trait C extends B {
  println("C ConstructionCode")
  override protected def postConstructionCode: Unit = {
    super.postConstructionCode
    println("C PostConstructionCode")
  }
}
object Test {
  def main(args: Array[String]) {
    val c = new C {}
  }
}

This produces the output:

A ConstructionCode
B ConstructionCode
C ConstructionCode

But when the constructed "C" has a non-empty constructor, such as:

    val c = new C { println("New C ConstructionCode") }

Then the DelayedInit works and generates:

A ConstructionCode
B ConstructionCode
C ConstructionCode
New C ConstructionCode
A PostConstructionCode
B PostConstructionCode
C PostConstructionCode
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 7, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4680?orig=1
Reporter: Richard Emberson (rmemberson)
Affected Versions: 2.9.1

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 10, 2011

Commit Message Bot (anonymous) said:
(extempore in r25067) A test case demonstrating some of the issues with DelayedInit.
References #4680. Review by odersky.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 12, 2011

@paulp said:
Raising the priority over the issues in the linked test case.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 21, 2012

@odersky said:
I think we'll get back to delayedInit once macros are a bit more mature. Demoting from critical to major.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

Matt Hicks (darkfrog) said:
Please come back and fix this. There's so much that DelayedInit makes possible but in its current state it's completely unusable.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

@paulp said:
I'm sure in martin's head DelayedInit is specified in spectacular detail; unfortunately in real life it is not, and I couldn't fix it if I wanted to because I don't know what it is supposed to do.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

Matt Hicks (darkfrog) said:
Paul, the concept is fairly simple. It allows the body / constructor of the class to be invoked explicitly within the delayedInit method receiving the "body" function that contains body of the class. This allows things to happen before or after the initialization of the class. This is of particular interest as it allows a class that is sub-classed to execute functionality post-construction of the instance. To my knowledge there is no other way to accomplish this in Java or Scala without DelayedInit.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

@paulp said:
It isn't the concept which needs documenting, it is the specification. For starters, please document exactly what should be the output of this:

https://raw.github.com/scala/scala/c8f4316b3/test/files/run/bug4680.scala

Here is what it produced at the time of its writing and presumably at present:

https://raw.github.com/scala/scala/c8f4316b3/test/files/run/bug4680.check

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

@paulp said:
Based on what you say you want, you would be better off with the alternative, which is also what I want, which I implemented and which was dismissed.

https://groups.google.com/forum/#!topic/scala-debate/5nbzkiv6lFI

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

Matt Hicks (darkfrog) said:
This is one of the bugs with DelayedInit. The first instantiation of C should have the same output as the second instantiation of C and similarly for the others.

I'm all for your alternative if we could get it introduced, but as it is DelayedInit exists now but requires bug-fixes...a new feature to the language I find doubtful at least in 2.10. I think it's reasonable for DelayedInit to be fixed before you can consider 2.10 to be out of RC considering this is a glaring bug that needs to be fixed.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

@paulp said:
It is not even a remote possibility this will be fixed for 2.10.0. You should probably set your sights on "ever".

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

@adriaanm said:
I agree with Paul. 2.10.0 is extremely nigh to final (as in, RC3 is closed and will hopefully become 2.10.0).

DelayedInit is unfortunately plagued with issues -- it seems to be common theme with these main-method replacements.
Maybe we should reconsider DelayedInit altogether, but that's a >= 2.11 thing.

In the meantime, yes, DelayedInit bugs should be fixed in 2.10.x

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

Matt Hicks (darkfrog) said:
Is there any other way to accomplish post-constructor functionality in Scala today without DelayedInit? I would be fine with Paul's alternative suggestion if that will get us where we need to be and then you could throw away DelayedInit for all I care. I'm sure I could use ASM to accomplish what I want, but that would require runtime bytecode manipulation and I'd like to avoid that.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Nov 20, 2012

@paulp said:
"Is there any other way to accomplish post-constructor functionality in Scala today without DelayedInit?"

Nope. I have another unpublished branch which comes at it from another angle; you can define postInit in the companion of any class, and after instantiation it traverses all the base classes calling any companion-defined postInit methods with the newly created instance as the argument. This makes it possible to inherit from a class, write custom postInit code, yet still have the base class's postInit code called, without being required to know about and dispatch to the existing postInits.

The major weakness is that it of necessity depends on the participation of the compiler; it would all break down if any such class were instantiated from java. This might not matter to most people, but it would be an issue nonetheless. (You could make the constructors private, but then you lose the part where you can still use inheritance.)

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 15, 2013

@adriaanm said:
Un-assigning to foster work stealing, as announced in https://groups.google.com/forum/?fromgroups=#!topic/scala-internals/o8WG4plpNkw

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented May 21, 2013

Matt Hicks (darkfrog) said:
Seriously? Please fix this guys! It is the only way to accomplish certain tasks and I can't imagine it should be that complex to solve.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented May 21, 2013

@paulp said:
Nobody is working on DelayedInit bugs. I couldn't even if I were willing, because I still don't know what it's supposed to do. But I think it is broken by design. In any case, there is little chance of it being fixed. The best I can do is work harder to prevent broken-by-design from being checked in.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented May 21, 2013

Matt Hicks (darkfrog) said:
Paul, I don't believe anyone would complain if DelayedInit never gets fixed if something like a PostInit trait were introduced that just had a postInit method that gets invoked after the class constructor has fully executed. That would be far more conceptually simple and I would imagine very easy to write. In fact, once Macros have fully made it into the system it could probably be accomplished with that.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 10, 2013

@adriaanm said:
Unassigning and rescheduling to M6 as previous deadline was missed.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 27, 2014

Matt Hicks (darkfrog) said:
If I have one major frustration with the Scala dev team it's that you often seem too busy adding new stuff that you don't seem to care enough to fix serious bugs in the system like this. Either remove it or fix it. Leaving something like this that you know for a fact to be broken release after release is extremely unprofessional.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 27, 2014

@adriaanm said:
We've not added any new stuff in Scala 2.11. We've focussed on making it easier to maintain Scala and contribute to it, by cleaning up our infrastructure, modularizing the library (and starting to modularize the compiler).

Fixing DelayedInit would likely require deprecating it and replacing it by a new feature, possibly the one proposed in #4330.
As you know, this is quite a tricky problem that has been (re)-designed several times.

Scala is an open source project. We do our best to facilitate and support community contributions, but we can't do everything ourselves as our resources are limited.
If you'd like to see this issue resolved, I suggest you team up with other potential users, write up a proposal and submit a PR.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 27, 2014

@paulp said:
Don't blame the whole team. I abandoned scala because I feel exactly the same way. I have given that speech essentially verbatim ("fix it or remove it - it is ridiculously unprofessional to knowingly ship broken code", etc.) probably a dozen times over the last few years. Never made any difference: scala still ships all the same broken code. Much of it has been broken in the same way since I started working on scala, that being when 2.7.5 was the current release.

For instance: https://groups.google.com/forum/#!topic/scala-debate/M8s8FmASL8Y

Trust me when I say DelayedInit is way, way, way down the list of things which are known to be broken and which will not be fixed.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 28, 2014

Matt Hicks (darkfrog) said:
Adriaan, Paul Phillips suggested a good viable alternative to DelayedInit over a year ago. The idea of a simple "postInit" method that would be invoked after the constructor is called seems on the surface to be a fairly simple solution and would be a fair replacement for DelayedInit in my opinion.

Paul, though I agree with your premise, I don't believe it is a sufficient problem on its own to abandon the language. I refuse to go back to Java and I don't see any other languages offering the capabilities that Scala does without the headaches caused by things like this.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 28, 2014

@adriaanm said:
I'm aware of the alternative -- I linked to it. We are a team of 4 (used to be 5), so we just can't do it all.
We will deprecate DelayedInit for 2.11, despite the fact that there's no alternative. Thanks for bringing it up again.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 28, 2014

Matt Hicks (darkfrog) said:
Adriaan, I understand you guys have a ton of work to do, but is it really so complicated to add this feature? This is something that simply isn't possible (at least to my knowledge) outside of the language, so it seems like you've opened the door a crack with DelayedInit and created the potential for so many useful things, but now you're slamming the door in our face until some ethereal time down the road when you can get around to it.

Is there something I can do to help? I would happily contribute code (and perhaps Paul might be persuaded to do so as well) if there were a high likelihood that it would find itself into a next release. I'm also happy to move to Switzerland if it will help. :-p

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 28, 2014

@paulp said:
Don't look to me - I already implemented what is described at https://groups.google.com/forum/#!topic/scala-debate/5nbzkiv6lFI and it does not seem to be among the code I managed to rescue for future reference in https://github.com/paulp/scala/tree/orphaned so I guess I took the hint when it was completely ignored that it was of no interest.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 28, 2014

@paulp said:
Quoting myself in that thread:

This is in that category of things where we suffer inordinately, for
years in some cases, over things which can trivially be fixed.
Sometimes it's worth doing something just for the something, not for a
large universe of abstract somethings, and especially when trying for
the large universe of abstract somethings leads to an inferior
solution for the specific actual something everyone wanted back in
Concretia.

Sigh.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 28, 2014

@paulp said:
All that said, re "but is it really so complicated to add this feature?", the answer is yes. That argument carries no weight at all; that you can even word the question that way illustrates that you don't begin to appreciate the complications.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 28, 2014

@paulp said:
https://groups.google.com/forum/#!topic/scala-sips/Li3XPbqCacE

"One post by one author." I'm glad to be reminded of this. There's so much which went into why I quit, I forget most of it at any given time. People really have no idea.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 28, 2014

Matt Hicks (darkfrog) said:
Paul, I know this isn't the right forum to ask, but what language have you switched to in replacement of Scala? Though there are things like this that really bother me about Scala, I still find that the benefits far outweigh the negatives especially when I start evaluating other languages.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jan 28, 2014

@paulp said:
I haven't. I'm partway through the long survey of alternatives which probably ends with me throwing my life away and starting from scratch.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Feb 16, 2016

Markus Marvell (Marvell) said (edited on Feb 21, 2016 10:00:34 AM UTC):
The idea behind DelayedInit is great. It has one advantage over suggested PostConstructor. It calls each superclass constructor body on the way to leaf subclass. For example we can design a safe object that doesn't break execution when constructor (one of in hierarchy) throws an exception. Instead it encapsulates a state that can be queried before using it.

trait Worker {
	def use(): Unit
}

trait SafeWorker extends DelayedInit with Worker{
	private[this] var ok = true
	override final def delayedInit(body:  Unit): Unit = {
		if (ok) try body catch {case e: Throwable  ok = false}
	}
	def isOk: Boolean = ok
	def use(): Unit = if (isOk) println(s"Working...") else println(s"Oops...")
}

class MediaSafeWorker extends SafeWorker {
	throw new Exception
}

class MySafeWorker extends MediaSafeWorker { () }

class UnsafeWorker extends Worker{
	throw new Exception
	def use(): Unit = println(s"Working...")
}

Running this code prints: "Oops..."

	val safeObj = new MySafeWorker
	safeObj.use()

But running this code breaks execution with exception.

	val unsafeObj = new UnsafeWorker
	unsafeObj.use()

(i) SUGGESTED OPTION

DelayedInit trait needs some improvements.

  • It skips empty constructor body as mentioned in this thread. It would be great if it will pass to delayedInit method even empty body.
  • @erik Westra suggested brilliant idea to detect class of currently executing constructor body. And it would be great to have delayedInit method version with extra argument for explicit current body class.
def delayedInit (body: => Unit, bodyClass: Class[_])

. Or even version with argument for explicit mark indicating leaf class body.

def delayedInit (body: => Unit, bodyClass: Class[_], isLeafClass: Boolean)

Or add def onCreate() when object construction is complete.

Current workaround is to use the solution suggested by @erik Westra for detection of leaf class. And slightly raw solution to make asynchronous delayed cancellable invokation of onCreate method in base superclass and in the end of each body of delayedInit method.

trait Obj extends DelayedInit {
	postOnCreate()
	override final def delayedInit(body:  Unit): Unit = {
		cancelPostOnCreate()// pseudocode
		body
		if ((body _).getClass.getDeclaringClass == getClass) onCreate()
		else postOnCreate()
	}
	def postOnCreate(): Unit = post(id= "onCreate", delay=10 ms, onCreate())// pseudocode
	def onCreate(): Unit
}

If the body of leaf class is empty onCreate will be called 10 milliseconds after instance is constructed. Not cool but at least.

(!) NOTE for Android developers.
Seems better add this line in Proguard config to avoid java.lang.NoClassDefFoundError:

-keep class scala.DelayedInit
@scabug scabug added this to the Backlog milestone Apr 7, 2017
@SethTisue

This comment has been minimized.

Copy link
Member

@SethTisue SethTisue commented Mar 2, 2018

DelayedInit is deprecated. the discussion here could be revived on https://contributors.scala-lang.org

@SethTisue SethTisue closed this Mar 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.