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

Strange type error with some AnyRef specializations (but not others) #5500

Closed
scabug opened this issue Feb 18, 2012 · 18 comments
Closed

Strange type error with some AnyRef specializations (but not others) #5500

scabug opened this issue Feb 18, 2012 · 18 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Feb 18, 2012

I think this might be related to some of the issues uncovered in #5488.

The basic gist is that while AnyRef specialization is working for simple type combinations (e.g. @specialized(Int, AnyRef) it seems to be breaking for "full specialization". Here's the code:

import scala.{specialized => spec}

// specialized on Int/AnyRef only
class C1[@spec(Int, AnyRef) A, @spec(Int, AnyRef) B](v:A, w:B)

// specialized on everything
class C2[@spec(Unit, Boolean, Byte, Char, Short, Int, Long, Float, Double, AnyRef) A, @spec(Unit, Boolean, Byte, Char, Short, Int, Long, Float, Double, AnyRef) B](v:A, w:B)

// specialized on everything except AnyRef
class C3[@spec(Unit, Boolean, Byte, Char, Short, Int, Long, Float, Double) A, @spec(Unit, Boolean, Byte, Char, Short, Int, Long, Float, Double) B](v:A, w:B)

object Test {
  def main(args:Array[String]) {
    // as expected, this specializes properly to C1$mcLI$sp
    println(new C1("abc", 123).getClass.getName)

    // this tries to use C2$mcLI$sp but fails to compile due to a bug
    println(new C2("abc", 123).getClass.getName)
    //test.scala:18: error: type mismatch;
    // found   : String("abc")
    // required: A$sp
    //    println(new C3("abc", 123).getClass.getName) // explodes
    //            ^
    //one error found

    // as expected, this uses the generic C3
    println(new C3("abc", 123).getClass.getName)
  }
}

Commenting out the second println allows the whole thing to compile and run as expected.

This error crops up when substituting C2$mcLI$sp for C2 in the tree, and the typing the new tree. I'm not sure why it works for C1 but not for C2. The trees look the same, at least as far as I can tell (e.g. using log output). Probably I need to figure out how to hook up a debugger and take a closer look.

@scabug
Copy link
Author

scabug commented Feb 18, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5500?orig=1
Reporter: @non
Other Milestones: 2.10.0

@scabug
Copy link
Author

scabug commented Feb 18, 2012

@non said (edited on Feb 18, 2012 7:16:55 PM UTC):
Well, at this point I have traced the problem back to the typer incorrectly deciding the type of C2$mcLI$sp's constructor, despite getting it right in the case of C1$mcLI$sp. The difference between C1 and C2 is that the typer substitutes String for A$sp whereas in C2 it doesn't.

The relevant typer debug output of C1 looks like:

    typing new <empty>.this.C1$mcLI$sp[lang.this.String].<init>("abc", 123): pt = ?: undetparams=, implicitsEnabled=false, silent=false, context.owner=method main
[log explicitouter] [context] ++ test.scala / Apply
        typing new <empty>.this.C1$mcLI$sp[lang.this.String].<init>: pt = ?: undetparams=, implicitsEnabled=false, silent=true, context.owner=method main
            typing new <empty>.this.C1$mcLI$sp[lang.this.String]: pt = ?: undetparams=, implicitsEnabled=false, silent=true, context.owner=method main
            typed new <empty>.this.C1$mcLI$sp[lang.this.String]: <empty>.this.C1$mcLI$sp[lang.this.String]
            adapted new <empty>.this.C1$mcLI$sp[lang.this.String]: <empty>.this.C1$mcLI$sp[lang.this.String] to ?, 
        typed new <empty>.this.C1$mcLI$sp[lang.this.String].<init>: (<param> v: lang.this.String, <param> w: scala.this.Int)<empty>.this.C1$mcLI$sp[lang.this.String]

The broken typer output of C2 looks like:

    typing new <empty>.this.C2$mcLI$sp[lang.this.String].<init>("abc", 123): pt = ?: undetparams=, implicitsEnabled=false, silent=false, context.owner=method main
[log explicitouter] [context] ++ test.scala / Apply
        typing new <empty>.this.C2$mcLI$sp[lang.this.String].<init>: pt = ?: undetparams=, implicitsEnabled=false, silent=true, context.owner=method main
            typing new <empty>.this.C2$mcLI$sp[lang.this.String]: pt = ?: undetparams=, implicitsEnabled=false, silent=true, context.owner=method main
            typed new <empty>.this.C2$mcLI$sp[lang.this.String]: <empty>.this.C2$mcLI$sp[lang.this.String]
            adapted new <empty>.this.C2$mcLI$sp[lang.this.String]: <empty>.this.C2$mcLI$sp[lang.this.String] to ?, 
        typed new <empty>.this.C2$mcLI$sp[lang.this.String].<init>: (<param> v: A$sp, <param> w: scala.this.Int)<empty>.this.C2$mcLI$sp[A$sp]

I'm still trying to understand how this all works, so I don't have any futher insight. But up to this point the trees seem identical (AFAIK) so I assume that some internal state somewhere else is what is different.

@scabug
Copy link
Author

scabug commented Feb 18, 2012

@non said (edited on Feb 18, 2012 9:53:32 PM UTC):
So I have tracked the divergence down to a different value of owntype being found in checkAccessible(). Still working on figuring out why that happens.

EDIT: This is taking me way too long, but I have traced through to asSeenFrom.

@scabug
Copy link
Author

scabug commented Feb 18, 2012

@paulp said:
I trust you saw what I checked in with the fix for 5488 in 1e648c3862

Comment found in test/files/run/t5488-fn.scala

/**  If the return types are specialized on AnyRef as well:

files/run/t5488-fn.scala:18: error: type mismatch;
 found   : Unit => String
 required: Unit => B$sp
    show(new B((x: Unit) => "abc"))
         ^
files/run/t5488-fn.scala:24: error: type mismatch;
 found   : (Int, Object) => String
 required: (Int, B$sp) => C$sp
    show(new C((x: Int, y: AnyRef) => "abc"))
         ^
files/run/t5488-fn.scala:26: error: type mismatch;
 found   : (Object, Int) => String
 required: (A$sp, Int) => C$sp
    show(new C((x: AnyRef, y: Int) => "abc"))
         ^
three errors found
**/

@scabug
Copy link
Author

scabug commented Feb 18, 2012

@paulp said:
Re "This is taking me way too long" welcome to my world! Plenty of room for more masochists!

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@paulp said:
This is enough to bring it out:

class C1A[
  @spec(Int, AnyRef) A,
  @spec(Int, AnyRef, Double) B
](v:A, w:B)

./a.scala:30: error: type mismatch;
 found   : String("abc")
 required: A$sp
    println(new C1A("abc", 123).getClass.getName)
            ^
one error found

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@non said:
Yeah, I saw that. Strangely, @SPEC(Unit, Int, AnyRef) seems to work fine... I wonder why that is.

Took a break for dinner but am back on this bug. Do you have any advice for running scalac in a debugger?

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@paulp said:
Ha ha, I've never done it.

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@non said:
Well, that makes me feel slightly less bad about my println-based debugging strategy.

I'm not sure this qualifies as a break though, but I am now seeing something that explains what's going on. Basically, asSeenFrom uses the AsSeenFromMap whose apply() methods calls itself recursively for awhile until it finds something it can return.

The problem is that for C2 if you follow the chain of calls down you end up finding a TypeRef(prefix, sym, args) whose sym is actually C2$mcLZ$sp.A$sp (notice the Z not the I). Here's the debugging output:

TypeRef prefix=<noprefix> sym=type A$sp/C2$mcLZ$sp.A$sp args=List()

This is the problem... it's somehow got ahold of the wrong A$sp so from there on down things explode. That's probably why it works for @SPEC(Int, AnyRef)... in those cases it doesn't find the "wrong" class' A$sp type param.

I don't know where this gets us, but it's something.

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@non said (edited on Feb 19, 2012 4:16:56 AM UTC):
OK, I'm pretty sure the issue is that makeSpecializedMembers/specName is linking the type params to the wrong class. For example:

[log explicitouter] creating tree for C2$mcLV$sp.<init>
[log explicitouter] specName(value v) env: Map(type A -> A$sp, type B -> scala.this.Unit) tvars: Set(type A)
[log explicitouter] specName #2 Map(C2.A -> C2$mcLZ$sp.A$sp, C2.B -> scala.Unit)
[log explicitouter] specName(value w) env: Map(type A -> A$sp, type B -> scala.this.Unit) tvars: Set(type B)
[log explicitouter] specName #2 Map(C2.A -> C2$mcLZ$sp.A$sp, C2.B -> scala.Unit)

(I added the "specName #2" log line which maps the value TypeRef into its symbol's fullName)

Notice that we're interested in C2$mcLV$sp but we're getting an A$sp owned by C2$mcLZ$sp (Z instead of V).

OK... now all we need to do is figure out what needs to be fixed to make specName return the right thing (which might include passing it more context).

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@non said:
YES! I have found the problem. As is often the case, it's a broken cache.

typeParamSubAnyRef uses the anyrefSpecCache. However, it only caches on the basis of sym, when really it needs to cache on the basis of both sym and clazz, since they are both params (and because otherwise this bug happens).

When I disabled the cache, the program worked correctly. I'm going to try to update the cache and then submit a patch for this.

Side-note: I need to learn to do this faster, 1 bug in 13 hours is pretty bad.

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@non said:
I have submitted a pull request that includes the test case here.

Case closed.

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@paulp said:
"Side-note: I need to learn to do this faster, 1 bug in 13 hours is pretty bad."

At 13 hours, you're so far ahead of the curve it's not even funny.

The reason those A$sp are owned by the wrong class is that they're not being cloned in produceTypeParameters. It looks like it is operating under the belief that it can avoid cloning them more frequently than it actually can. I think the cloning part should look more like this (after renaming variables to be less opaque.)

    val clonedParams = tparams map (tparam =>
      env get tparam filter (_.typeSymbol.owner == newOwner) match {
        case Some(tp) => tp.typeSymbol
        case _        => tparam cloneSymbol newOwner
      }
    )

Of course, it shouldn't look like that either, because there is WAY TOO MUCH low-level cloning and most of it is done wrong in one way or another, which is one reason why NSDNHO is not a meaningless string of letters to us like it ought to be.

There's still plenty of bug to be had around here even if that cache change works out. Stay with it, it's very exciting to have anyone else actively working on stuff like this.

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@paulp said:
1df4fc6e59 and ec160bae7e

@scabug scabug closed this as completed Feb 19, 2012
@scabug
Copy link
Author

scabug commented Feb 19, 2012

@retronym said:

Do you have any advice for running scalac in a debugger?

Ha ha, I've never done it.

I find it useful, and pretty straightforward. Maybe it can trim an hour or two from your efforts. Here's a guide I prepared: Debugging Scalac in five minutes flat:

https://gist.github.com/863884

You can do something similar with an IntelliJ project based on the checked-out sources of scala; in that case I tend to add -Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005 to JAVA_OPTS before running scalac, then connect to it with a Remote Debug configuration.

http://www.jetbrains.com/idea/webhelp/run-debug-configuration-remote.html

Of course the biggest time saver is to whittle down a five line reproduction before diving into the compiler internals, but I think you've already internalized this life skill.

@scabug
Copy link
Author

scabug commented Feb 19, 2012

@retronym said:
As an aside, this reminds me of tracking down a similar over-optimistic caching problem with implicits way back when. Now that all the caches are centralized in perRunCaches, it might be feasible to add -Ycache-disable as a generic troubleshooting option.

@scabug
Copy link
Author

scabug commented Feb 20, 2012

@paulp said:
Most of the maps created via perRunCaches are not performance optimizations, they are necessary for correctness. They're typically of the form "I need to find this thing later and do one more thing to it after some other thing has happened." The "per-run" aspect is to protect the resident compiler from leaks they may/do contain.

@scabug
Copy link
Author

scabug commented Feb 20, 2012

@retronym said:
Ah, good to know. Thanks for the explanation.

@scabug scabug added this to the 2.10.0-M1 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants