can you fix the f-f-f-f-failing test

Feature interactions between checkInit and optimization -- I fear that
one's for you, Iulian!

On Wed, Oct 5, 2011 at 11:52 AM, martin odersky wrote:
> Feature interactions between checkInit and optimization -- I fear that
> one's for you, Iulian!

Hm.. I haven't committed anything to optimizations or checkinit for a
year at least.. But I'll have a look at some point next week

iulian

>
>  -- Martin
>
> On Wed, Oct 5, 2011 at 6:25 AM, Paul Phillips wrote:
>> If anyone's looking for an interesting bug to figure out, I am feeling
>> generous so I'm throwing one of the good ones your way.
>>
>>  https://scala-webapps.epfl.ch/jenkins/job/scala-nightly-checkinit/
>>
>> This is the -Xcheckinit build, which generates a little bonus bytecode
>> to throw an exception when an uninitialized value is accessed.
>> Usually a failure here means val A needs to come before val B, not
>> after, or something like that.  We have one test failing for the last
>> week.  "Yeah, real interesting, let me just cancel my lunch date with
>> lady gaga and the ghost of david foster wallace and I'll be right
>> there" you are saying.  But here is the thing: it only fails if the
>> compiler which builds the test was itself built with -optimise.  (The
>> test is not being compiled with -optimise.  The compiler building the
>> test, it is what is compiled with -optimise.)
>>
>> So really there are between one and two bugs: there must be a bug in
>> either -optimise or the absence of -optimise, since the scalacs they
>> are building behave differently.  And then depending on who is wrong,
>> there is also a bug in the failing test, or there isn't.
>>
>
>
>
> --
> Martin Odersky
> Prof., EPFL and Chairman, Typesafe
> PSED, 1015 Lausanne, Switzerland
> Tel. EPFL: +41 21 693 6863
> Tel. Typesafe: +41 21 691 4967
>

On Sun, Oct 9, 2011 at 12:13 AM, iulian dragos wrote:
> Hm.. I haven't committed anything to optimizations or checkinit for a
> year at least.. But I'll have a look at some point next week

I may have seen through the haze by rubber ducky debugging (I started
writing this email with no idea.)

As everyone has probably figured out by now, definitions.init for a
compiler instance is called when you instantiate the first Global#Run.

https://github.com/scala/scala/blob/master/src/compiler/scala/tools/nsc/...

In this case, during the construction of definitions.init, type
completion comes back around, and in scala.reflect.runtime.Loaders we
have

class TopClassCompleter(clazz: Symbol, module: Symbol) extends SymLoader {
override def complete(sym: Symbol) = {
atPhaseNotLaterThan(picklerPhase) { ...

That reference to picklerPhase is to a val in the Run instance which
is later in the constructor than the point where definitions.init is
called. So, that explains the uninitialized part, but not why it only
happens under -optimise.

Looking at the function being called, things start coming into focus:

@inline final def atPhaseNotLaterThan[T](target: Phase)(op: => T): T =
if (target != null && phase.id > target.id) atPhase(target)(op) else op

So under -optimise this is inlined into TopClassCompleter#complete.
This must interact with -Xcheckinit in such a way that the exception
is triggered on the inlined version, but under normal compilation the
null check on target avoids it. It seems like if it is null, it
should be triggered regardless of inlining, because it is an eager
parameter to atPhaseNotLaterThan, and it has to be read to be passed.
A bug in Xcheckinit perhaps.

On Mon, Oct 10, 2011 at 11:13 AM, Paul Phillips wrote:
> On Sun, Oct 9, 2011 at 12:13 AM, iulian dragos wrote:
>> Hm.. I haven't committed anything to optimizations or checkinit for a
>> year at least.. But I'll have a look at some point next week
>
> I may have seen through the haze by rubber ducky debugging (I started
> writing this email with no idea.)
>
> As everyone has probably figured out by now, definitions.init for a
> compiler instance is called when you instantiate the first Global#Run.
>
>  https://github.com/scala/scala/blob/master/src/compiler/scala/tools/nsc/Global.scala#L775
>
> In this case, during the construction of definitions.init, type
> completion comes back around, and in scala.reflect.runtime.Loaders we
> have
>
>  class TopClassCompleter(clazz: Symbol, module: Symbol) extends SymLoader {
>    override def complete(sym: Symbol) = {
>      atPhaseNotLaterThan(picklerPhase) { ...
>
> That reference to picklerPhase is to a val in the Run instance which
> is later in the constructor than the point where definitions.init is
> called.  So, that explains the uninitialized part, but not why it only
> happens under -optimise.

It doesn't. For me it fails just fine with a plain build, with -Xcheckinit.

>
> Looking at the function being called, things start coming into focus:
>
>  @inline final def atPhaseNotLaterThan[T](target: Phase)(op: => T): T =
>    if (target != null && phase.id > target.id) atPhase(target)(op) else op
>
> So under -optimise this is inlined into TopClassCompleter#complete.
> This must interact with -Xcheckinit in such a way that the exception
> is triggered on the inlined version, but under normal compilation the
> null check on target avoids it.  It seems like if it is null, it
> should be triggered regardless of inlining, because it is an eager
> parameter to atPhaseNotLaterThan, and it has to be read to be passed.
> A bug in Xcheckinit perhaps.
>

On Tue, Oct 25, 2011 at 6:42 AM, iulian dragos wrote:
> It doesn't. For me it fails just fine with a plain build, with -Xcheckinit.

Oh. (I like that, "fails just fine." All failures are working
correctly!) I guess I bobbled a testing ball then. That's strange,
usually if I email about something I am pretty sure about it. OK, if
it isn't affected by compiler options then we can feel good about
blaming the test itself.

On Tue, Oct 25, 2011 at 4:03 PM, Paul Phillips wrote:
> On Tue, Oct 25, 2011 at 6:42 AM, iulian dragos wrote:
>> It doesn't. For me it fails just fine with a plain build, with -Xcheckinit.
>
> Oh.  (I like that, "fails just fine." All failures are working
> correctly!) I guess I bobbled a testing ball then.  That's strange,
> usually if I email about something I am pretty sure about it.  OK, if
> it isn't affected by compiler options then we can feel good about
> blaming the test itself.

I'm still looking at it, but I just build the compiler with
-Xcheckinit and it fails allright. I just built it with -optimize and
-Xcheckinit, and it failed again. I am building now with -optimize,
and I expect it to pass. If that doesn't happen, I'm in trouble :)

iulian

On Tue, Oct 25, 2011 at 4:03 PM, Paul Phillips wrote:
> On Tue, Oct 25, 2011 at 6:42 AM, iulian dragos wrote:
>> It doesn't. For me it fails just fine with a plain build, with -Xcheckinit.
>
> Oh.  (I like that, "fails just fine." All failures are working
> correctly!) I guess I bobbled a testing ball then.  That's strange,
> usually if I email about something I am pretty sure about it.  OK, if
> it isn't affected by compiler options then we can feel good about
> blaming the test itself.
>

It could be this confusing section in build.xml?

note that nightly.checkinit does NOT pass -optimise, but it uses a
variable called optimise to pass -Xcheckinit. A little bit obfuscated.

iulian

On Tue, Oct 25, 2011 at 7:24 AM, iulian dragos wrote:
> note that nightly.checkinit does NOT pass -optimise, but it uses a
> variable called optimise to pass -Xcheckinit. A little bit obfuscated.

Obfuscated, heavens no. In an unrelated matter, I find stodgy old
"Int" too constraining. I'm spicing it up!

scala> def f(x: Int, y: Int): Int = x + y + 1238.11223f
f: (x: Int, y: Int)Int

On Tue, Oct 25, 2011 at 4:17 PM, iulian dragos wrote:
> On Tue, Oct 25, 2011 at 4:03 PM, Paul Phillips wrote:
>> On Tue, Oct 25, 2011 at 6:42 AM, iulian dragos wrote:
>>> It doesn't. For me it fails just fine with a plain build, with -Xcheckinit.
>>
>> Oh.  (I like that, "fails just fine." All failures are working
>> correctly!) I guess I bobbled a testing ball then.  That's strange,
>> usually if I email about something I am pretty sure about it.  OK, if
>> it isn't affected by compiler options then we can feel good about
>> blaming the test itself.
>
> I'm still looking at it, but I just build the compiler with
> -Xcheckinit and it fails allright. I just built it with -optimize and
> -Xcheckinit, and it failed again. I am building now with -optimize,
> and I expect it to pass. If that doesn't happen, I'm in trouble :)

Ok, I tried the following combinations:

-optimize -> no exception
-Xcheckinit -> UninitializedFieldError
-Xcheckinit -optimize -> UninitializedFieldError

It looks like a genuine error, and that field is indeed accessed
before being initialized. The null check you spotted makes the test
pass when there's no -Xcheckinit, and it makes me think that it is
intentional. If reading that field before it's initialized is
expected, we should make it a var or a lazy val, depending on how that
field is used.

At any rate, the conclusion is that the error is correct, and neither
the optimizer nor checkinit are at fault. I let someone who knows
better how that field has to operate fix the underlying problem.

cheers,
iulian

>
> iulian
>
>
>
> --
> « Je déteste la montagne, ça cache le paysage »
> Alphonse Allais
>