Skip to content

Method of specialized class does not delegate properly to specialized method of same class #6237

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

Closed
scabug opened this issue Aug 15, 2012 · 11 comments
Assignees

Comments

@scabug
Copy link

scabug commented Aug 15, 2012

Best to show a sample. This was checked against 2.10.x and master

class Foo[@specialized(Int) A] {
  def barA(a: A): A = barB[A](a)
  def barB[@specialized(Int) B](b: B): B = b
}

object Test {
  def main(args: Array[String]) {
    var foo = new Foo[Int]
    println(foo.barA(1))
    println(foo.barB(2))
  }
}

Checking javap -verbose Foo, we see that barA$mcI$sp calls its generic counterpart.

public int barA$mcI$sp(int);
  Code:
   Stack=2, Locals=2, Args_size=2
   0:   aload_0
   1:   iload_1
   2:   invokestatic    #27; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
   5:   invokevirtual   #29; //Method barA:(Ljava/lang/Object;)Ljava/lang/Object;
   8:   invokestatic    #33; //Method scala/runtime/BoxesRunTime.unboxToInt:(Ljava/lang/Object;)I
   11:  ireturn
  LocalVariableTable: 
   Start  Length  Slot  Name   Signature
   0      12      0    this       LFoo;
   0      12      1    a       I

  LineNumberTable: 
   line 2: 0
@scabug
Copy link
Author

scabug commented Aug 15, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6237?orig=1
Reporter: Adam Klein (aklein-at-novus.com)
Affected Versions: 2.10.0

@scabug
Copy link
Author

scabug commented Aug 15, 2012

@non said:
Hi, I think you're misunderstanding the machinery here.

You'll want to be looking at the bytecode for the Foo$mcI$sp class if you want to see Foo specialized on Int.

When I do that, I see:

public int barA$mcI$sp(int);
  Code:
   0:   aload_0
   1:   iload_1
   2:   invokevirtual   #18; //Method Foo.barB$mIc$sp:(I)I
   5:   ireturn

So it looks to me like Foo[Int] does have a correct specialized implementation. Of course, since the Foo class is the "generic version" of the class, it has to call the generic version of the method as well in the specialized case (the case where someone calls barA$mcI$sp on a Foo which itself boxes).

@scabug
Copy link
Author

scabug commented Aug 15, 2012

Adam Klein (aklein-at-novus.com) said:
Got it - I see that I should be checking Foo$mcI$sp (ie javap -verbose Foo$mcI$sp). But strangely, when I do so I still see the following (using scalac commit 1c5526ed81a27550cc4f376d6debe89216470c51). Am I missing something?

  public int barA$mcI$sp(int);
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0       
         1: iload_1       
         2: invokestatic  #27                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
         5: invokevirtual #29                 // Method barA:(Ljava/lang/Object;)Ljava/lang/Object;
         8: invokestatic  #33                 // Method scala/runtime/BoxesRunTime.unboxToInt:(Ljava/lang/Object;)I
        11: ireturn       

@scabug
Copy link
Author

scabug commented Aug 15, 2012

@non said:
I can't replicate that... I'm still seeing the "correct" version of barA$mcI$sp. What (if any) compiler flags are you using?

Here are the steps I'm taking:

$ rm -f *.class
$ ~/w/scala/build/quick/bin/scalac test.scala
$ javap -c 'Foo$mcI$sp' | grep -A 6 'int barA$mcI$sp'
public int barA$mcI$sp(int);  Code:
   0:   aload_0
   1:   iload_1   2:   invokevirtual   #18; //Method Foo.barB$mIc$sp:(I)I
   5:   ireturn

This is with the 2.10.x as of last night. Could you be using a different scalac than you expect?

@scabug
Copy link
Author

scabug commented Aug 15, 2012

Adam Klein (aklein-at-novus.com) said:
Ah, stupid mistake on my part. I didn't escape the '$' character in the terminal, so it must have been printing out the bytecode for the generic class. I now see what you are seeing.

One final question and I'll close the issue: in the main method of the Test$ class, it is calling Foo.barA$mcI$sp on the instance of Foo$mcI$sp. Just checking this should dispatch to the specialized method of the specialized class (presumably that's what invokevirtual #28 implies)?

  public void main(java.lang.String[]);
    Code:
       0: new           #16                 // class Foo$mcI$sp
       3: dup           
       4: invokespecial #17                 // Method Foo$mcI$sp."<init>":()V
       7: astore_2      
       8: getstatic     #22                 // Field scala/Predef$.MODULE$:Lscala/Predef$;
      11: aload_2       
      12: iconst_1      
      13: invokevirtual #28                 // Method Foo.barA$mcI$sp:(I)I
      16: invokestatic  #34                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
      19: invokevirtual #38                 // Method scala/Predef$.println:(Ljava/lang/Object;)V
      22: getstatic     #22                 // Field scala/Predef$.MODULE$:Lscala/Predef$;
      25: aload_2       
      26: iconst_2      
      27: invokevirtual #41                 // Method Foo.barB$mIc$sp:(I)I
      30: invokestatic  #34                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
      33: invokevirtual #38                 // Method scala/Predef$.println:(Ljava/lang/Object;)V
      36: return   

@scabug scabug closed this as completed Aug 15, 2012
@scabug
Copy link
Author

scabug commented Aug 15, 2012

@non said:
Yes, the invokevirtual call will find the correct implementation for the given instance's class.

This is why the generic version needs to have all the specialized method variants defined: so that code can call those methods on instances (parameterized on the right type) which may or may not be specialized.

@scabug
Copy link
Author

scabug commented Aug 15, 2012

Adam Klein (aklein-at-novus.com) said:
Ok, I'm finally able to reproduce what I was originally seeing (but not without some additional complexity).

trait Foo[@specialized(Int) A] {
  def foo[@specialized(Int) B](init: B)(f: (B, A) => B): B = init
}

class Bar[@specialized(Int) A] extends Foo[A] {
  def bar1(init: A)(f: (A, A) => A): A = foo[A](init)(f)
  def bar2(init: A)(f: (A, A) => A): A = super.foo[A](init)(f)
}

In Bar$mcI$sp, we see the following: method bar2 delegates to the abstract base class specialized method Foo$mcI$sp$class.foo$mIcI$sp, without boxing; whereas method bar1 delegates to the inherited Foo$mcI$sp.foo (non-specialized) method. How is 'super' achieving this here, and is this a bug?

 public int bar1$mcI$sp(int, scala.Function2<java.lang.Object, java.lang.Object, java.lang.Object>);
    Code:
       0: aload_0       
       1: iload_1       
       2: invokestatic  #43                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
       5: aload_2       
       6: invokeinterface #45,  3           // InterfaceMethod Foo$mcI$sp.foo:(Ljava/lang/Object;Lscala/Function2;)Ljava/lang/Object;
      11: invokestatic  #49                 // Method scala/runtime/BoxesRunTime.unboxToInt:(Ljava/lang/Object;)I
      14: ireturn       
    
  public int bar2$mcI$sp(int, scala.Function2<java.lang.Object, java.lang.Object, java.lang.Object>);
    Code:
       0: aload_0       
       1: iload_1       
       2: aload_2       
       3: invokestatic  #33                 // Method Foo$mcI$sp$class.foo$mIcI$sp:(LFoo$mcI$sp;ILscala/Function2;)I
       6: ireturn       

@scabug
Copy link
Author

scabug commented Aug 15, 2012

Adam Klein (aklein-at-novus.com) said:
I've come up with a similarly puzzling case without inheritance.

class Foo[@specialized(Int) A] {
  def foo[@specialized(Int) B](f: Function2[B, A, B])(b: B, a: A): B = f(b, a)
}

class Bar[@specialized(Int) A] {
  val aFoo = new Foo[A]
  def fun(a: A, b: A): A = a
  def bar(a: A, b: A): A = aFoo.foo(fun)(a, b)
}

In the byte code, do I understand correctly that the foo invocation (invokevirtual #43) is invoking the non-specialized Foo class's (specialized) method? My profiling reveals boxing & unboxing.

public class Bar$mcI$sp extends Bar<java.lang.Object> {
  public final Foo<java.lang.Object> aFoo$mcI$sp;

  public int bar$mcI$sp(int, int);
    Code:
       0: aload_0       
       1: invokevirtual #31                 // Method aFoo:()LFoo;
       4: new           #33                 // class Bar$mcI$sp$$anonfun$bar$mcI$sp$1
       7: dup           
       8: aload_0       
       9: invokespecial #37                 // Method Bar$mcI$sp$$anonfun$bar$mcI$sp$1."<init>":(LBar$mcI$sp;)V
      12: iload_1       
      13: iload_2       
      14: invokevirtual #43                 // Method Foo.foo$mIcI$sp:(Lscala/Function2;II)I
      17: ireturn       
}

This is certainly undesirable, but is it somehow necessary?

@scabug
Copy link
Author

scabug commented Aug 16, 2012

Adam Klein (aklein-at-novus.com) said:
Looks like my most recent comment is a "rediscovery" of #5490. Not sure what's going on in the comment just prior to that (regarding the effect of 'super')

@scabug
Copy link
Author

scabug commented Aug 16, 2012

@non said:
I can totally replicate your first example, and that definitely points out a bug in specialization.

Using -Ydebug -Ylog:specialize and diffing the logs for the two methods I can see where they diverge, but I don't totally understand why (I think it will have something to do with the "" flag that appears in the "this" case but not the "super" case).

The two-second summary is that for the "this" case, by the time it starts looking at "foo" it has eagerly replaced the A's with Int, which means that when it goes to actually see if foo$mcI$sp is a valid method, it croaks because its A's have already been replaced. Sorry if that explanation isn't helpful, but I can't give better at the moment.

As we established earlier, you can't assume that Foo.foo$mIcI$sp is invoked on a non-specialized instance. I haven't worked through your second example, and I bet you're right about boxing, but the pasted bytecode doesn't establish anything.

@scabug
Copy link
Author

scabug commented Aug 16, 2012

@non said:
Could you open a separate bug for the "super" issue?

A lot of these specialization bugs get sort of conjoined in a weird way, making it hard to keep track of what the bug actually is. I'd prefer to let this one die (given that the subject is not really correct), and instead open a new one that specifically mentions super vs this.

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