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

Specialized classes with lazy vals generate broken variants #5552

Closed
scabug opened this issue Mar 6, 2012 · 8 comments

Comments

@scabug
Copy link

commented Mar 6, 2012

Here's the test case:

import scala.{specialized => spec}
class C[@spec(Int) A](a:A) {
  lazy val b = (a, a)
  def c = b
}
object Test {
  def main(args:Array[String]) {
    println(new C(3).c)
    println(new C(3.0).c)
  }
}

Expected output:

(3,3)
(3.0,3.0)

Actual output:

null
(3.0,3.0)

The problem seems to be that the specialized subclass doesn't get its own bitmap... it just assumes its lazy val field has already been initialized. I'm not really an expert on lazy val, but barring deeper changes (e.g. not duplicating vals between generic class and specialized subclass) I think we need to copy the bitmask and the code in b that checks it.

@scabug

This comment has been minimized.

Copy link
Author

commented Mar 6, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5552?orig=1
Reporter: @non
Attachments:

@scabug

This comment has been minimized.

Copy link
Author

commented Mar 27, 2012

@non said:
Oops, I forgot to update this.

It actually turns out that the LAZY flag was being masked by the specialization phase (Paul Phillips helped diagnose this). I'll try to find the patch which turns this off.

Once you fix this, things break farther down the line, and I haven't had much time to spend on it.

@scabug

This comment has been minimized.

Copy link
Author

commented May 11, 2012

@non said:
So, I have a small update.

As I said in the last comment, the LAZY flag was being removed. In addition, the ACCESSOR flag was also being removed. Once I got those fixed, I moved on to the next error (I have attached a patch that fixes that issue).

At that point, the problem is that C$mcI$sp.b$mcI$sp() isn't given the same "lazy method" block that C.b() has. This causes a MatchError in the mixin phase later. Compare:

// generic b() is fine
class C ... {
    <method> <stable> <accessor> lazy <triedcooking> def b(): scala.this.Tuple2[A,A] = {
      C.this.b = new scala.this.Tuple2[A,A].<init>(C.this.a, C.this.a);
      C.this.b
    };
}

// specialized b$mcI$sp() is broken
class C$mcI$sp ... {
    <method> <stable> <accessor> lazy <specialized> <triedcooking> def b$mcI$sp(): scala.this.Tuple2[scala.this.Int,scala.this.Int] = C$mcI$sp.this.b$mcI$sp;
}

I think that if we can get the body of b$mcI$sp() to look more like the body of b(), which really just means assigning a tuple to the b$mcI$sp field, then things might work.

Looking into this further.

@scabug

This comment has been minimized.

Copy link
Author

commented May 11, 2012

@non said:
This file preserves the LAZY and ACCESSOR flags which leads to the crash at mixin.

@scabug

This comment has been minimized.

Copy link
Author

commented May 11, 2012

@non said:
OK, sorry to keep spamming the comments but it helps to think "out loud".

At this point I think the ACCESSOR flag is confusing other parts of specialization. Namely, we need to be saving and copying over the method def, but instead we're treating this like other simple accessors.

I'm still looking into how to fix this.

@scabug

This comment has been minimized.

Copy link
Author

commented May 14, 2012

@non said:
Hurrah! I think I have this fixed.

Sending a pull request to Github now.

@scabug

This comment has been minimized.

Copy link
Author

commented May 14, 2012

@non said:
This was fixed in f7d5f45.

@scabug scabug closed this May 14, 2012

@scabug

This comment has been minimized.

Copy link
Author

commented Aug 19, 2016

@adriaanm said:
With scala/scala#5294, specialized lazy vals will behave like lazy vals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.