Scala Programming Language
  1. Scala Programming Language
  2. SI-4925

Inner class methods not inlined when compiled separately

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Not a Bug
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Misc Compiler, Optimizer
    • Labels:
      None

      Description

      A.scala
      class A {
        final class Inner {
          @inline def foo = 7
        }
        def inner = new Inner 
      }
      
      B.scala
      class B {
        def baz = {
          val a = new A
          val o = a.inner
          val z = o.foo
          println(z)
        }
      }
      

      Compile with:

      $ scalac A.scala
      $ scalac -optimize B.scala
      B.scala:8: warning: Could not inline required method foo because bytecode was not available.
          val z = o.foo
                    ^
      one warning found
      

      Expected behavior would be to inline foo, as correctly happens when both files are compiled together.

      Likely cause:

      ICodeReader overrides ClassfileParser.classNameToSymbol but does not replicate the logic that deals with inner classes (i.e. try innerClasses.get(name) object before trying definitions.getClass(name))

        Activity

        Hide
        Paul Phillips added a comment -

        Surprisingly, this bug seems to be a mirage. It's only due to their being in the empty package, and whether bug or not, empty package classes aren't visible through definitions.getClass. If I put them both in the same package there's no warning and I can see it inlined:

        public void baz();
          Code:
           Stack=2, Locals=2, Args_size=1
           0:	new	#7; //class bip/A
           3:	dup
           4:	invokespecial	#11; //Method bip/A."<init>":()V
           7:	astore_1
           8:	aload_1
           9:	invokevirtual	#15; //Method bip/A.inner:()Lbip/A$Inner;
           12:	pop
           13:	getstatic	#21; //Field scala/Predef$.MODULE$:Lscala/Predef$;
           16:	bipush	7
           18:	invokestatic	#27; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
           21:	invokevirtual	#31; //Method scala/Predef$.println:(Ljava/lang/Object;)V
           24:	return
        

        So I'm closing this, but if there is still an issue which is being obscured by the empty package thing let me know.

        Show
        Paul Phillips added a comment - Surprisingly, this bug seems to be a mirage. It's only due to their being in the empty package, and whether bug or not, empty package classes aren't visible through definitions.getClass. If I put them both in the same package there's no warning and I can see it inlined: public void baz(); Code: Stack=2, Locals=2, Args_size=1 0: new #7; //class bip/A 3: dup 4: invokespecial #11; //Method bip/A."<init>":()V 7: astore_1 8: aload_1 9: invokevirtual #15; //Method bip/A.inner:()Lbip/A$Inner; 12: pop 13: getstatic #21; //Field scala/Predef$.MODULE$:Lscala/Predef$; 16: bipush 7 18: invokestatic #27; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer; 21: invokevirtual #31; //Method scala/Predef$.println:(Ljava/lang/Object;)V 24: return So I'm closing this, but if there is still an issue which is being obscured by the empty package thing let me know.
        Hide
        Tiark Rompf added a comment -

        But it should still inline in the given case, don't you think? So I would tend to say it's still a bug, although less severe. (Btw, what happens if they reside in different packages?)

        Show
        Tiark Rompf added a comment - But it should still inline in the given case, don't you think? So I would tend to say it's still a bug, although less severe. (Btw, what happens if they reside in different packages?)
        Hide
        Paul Phillips added a comment -

        It works if they're in different packages, or if the inlining class is in the empty package. It's only if the callee is in the empty package it fails, because definitions.getClass will not admit to knowledge of empty package classes. It might be easy to fix, I'm just disinclined to work too hard on it since it's essentially cosmetic. (On the other hand I DO want to get trait methods inlining...)

        Show
        Paul Phillips added a comment - It works if they're in different packages, or if the inlining class is in the empty package. It's only if the callee is in the empty package it fails, because definitions.getClass will not admit to knowledge of empty package classes. It might be easy to fix, I'm just disinclined to work too hard on it since it's essentially cosmetic. (On the other hand I DO want to get trait methods inlining...)
        Hide
        Paul Phillips added a comment -

        Oh I see, you were right that it's that override, because the superclass has logic to deal specifically with this:

        if (name.pos('.') == name.length)
        definitions.getMember(definitions.EmptyPackageClass, name.toTypeName)
        else
        definitions.getClass(name) // see tickets #2464, #3756

        Show
        Paul Phillips added a comment - Oh I see, you were right that it's that override, because the superclass has logic to deal specifically with this: if (name.pos('.') == name.length) definitions.getMember(definitions.EmptyPackageClass, name.toTypeName) else definitions.getClass(name) // see tickets #2464, #3756
        Hide
        Paul Phillips added a comment -

        OK, fixed in 2820770bff .

        Show
        Paul Phillips added a comment - OK, fixed in 2820770bff .

          People

          • Assignee:
            Paul Phillips
            Reporter:
            Tiark Rompf
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development