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

Jump to bottom

optimizer: List + dropWhile = "Could not inline" warning #7529

Open
scabug opened this issue May 29, 2013 · 5 comments
Open

optimizer: List + dropWhile = "Could not inline" warning #7529

scabug opened this issue May 29, 2013 · 5 comments
Labels
optimizer

Comments

@scabug
Copy link

scabug commented May 29, 2013 

% cat O.scala
List(0).dropWhile(_ < 1)
% scala -optimize -Yinline-warnings O.scala
/Users/tisue/newbug/O.scala:1: warning: Could not inline required method dropWhile because access level required by callee not matched by caller.
List(0).dropWhile(_ < 1)
                 ^
one warning found

see also #6716
@scabug
Copy link
Author

scabug commented May 29, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7529?orig=1
Reporter: @SethTisue
Affected Versions: 2.11.0-RC1
See #6716

@scabug
Copy link
Author

scabug commented May 29, 2013 •
edited by retronym

@magarciaEPFL said:

The new optimizer explains in more detail ( -Ydebug -neo:o3 -Ylog:jvm -Yinline-warnings ) why that inlining is unfeasible.

Summary: because of the local method in List.dropWhile , which becomes private to immutable.List

Snippet:

class C {
  def m() {
    List(0).dropWhile(_ < 1)
  }
}

warning: Closure-inlining failed because
  scala/collection/immutable/List::dropWhile(Lscala/Function1;)Lscala/collection/immutable/List;
contains instruction 
  INVOKESPECIAL 
    scala/collection/immutable/List.loop$3
    (Lscala/collection/immutable/List;Lscala/Function1;)Lscala/collection/immutable/List;
that would cause IllegalAccessError from class C
    List(0).dropWhile(_ < 1)
                     ^
[log jvm] Failed closure-inlining Callsite:
            INVOKEVIRTUAL
            scala/collection/immutable/List.dropWhile
            (Lscala/Function1;)Lscala/collection/immutable/List; 
          in method C::m()V
[log jvm] Bytecode of callee, declared by scala/collection/immutable/List 

  // access flags 0x11
  // signature (Lscala/Function1<TA;Ljava/lang/Object;>;)Lscala/collection/immutable/List<TA;>;
  // declaration: scala.collection.immutable.List<A> dropWhile(scala.Function1<A, java.lang.Object>)
  public final dropWhile(Lscala/Function1;)Lscala/collection/immutable/List;
   L0
    LINENUMBER 284 L0
    ALOAD 0
    ALOAD 0
    ALOAD 1
    INVOKESPECIAL 
      scala/collection/immutable/List.loop$3 
     (Lscala/collection/immutable/List;Lscala/Function1;)Lscala/collection/immutable/List;
    ARETURN
   L1
    LOCALVARIABLE this Lscala/collection/immutable/List; L0 L1 0
    LOCALVARIABLE p Lscala/Function1; L0 L1 1
    MAXSTACK = 3
    MAXLOCALS = 2

Where does that private method come from? From the @tailrec-optimized loop():

  @inline final override def dropWhile(p: A => Boolean): List[A] = {
    @tailrec
    def loop(xs: List[A]): List[A] =
      if (xs.isEmpty || !p(xs.head)) xs
      else loop(xs.tail)

    loop(this)
  }

It's not a bug of the old optimizer.

@scabug
Copy link
Author

scabug commented Mar 18, 2014 •
edited by retronym

@SethTisue said:
This makes the combination of -optimize and -Xfatal-warnings pretty much unusable, since even if you don't explicitly pass -Yinline-warnings, the "re-run with -Yinline-warnings for details" warning itself is fatal.

I believe almost any well-managed project should be using -Xfatal-warnings, so this means I can't use the 2.11 optimizer on any of my projects.

@scabug
Copy link
Author

scabug commented Mar 18, 2014

@SethTisue said (edited on Nov 2, 2014 2:32:42 AM UTC):
#8410 is very similar, but for -deprecation instead of -Yinline-warnings. Jason Zaugg's comment is "For now, I'd suggest to run without fatal warnings and post-process the compiler output in a build tool."

@scabug
Copy link
Author

scabug commented Nov 20, 2015

@SethTisue said:
current status on 2.12.0-M3:

% scala212 -Yopt:l:classpath =(echo 'List(0).dropWhile(_ < 1)')
warning: there was one inliner warning; re-run with -Yopt-warnings for details
one warning found
% scala212 -Yopt:l:classpath -Yopt-warnings =(echo 'List(0).dropWhile(_ < 1)')
/tmp/zshzdt5Sn:1: warning: scala/collection/immutable/List::dropWhile(Lscala/Function1;)Lscala/collection/immutable/List; is annotated @inline but could not be inlined:
The callee scala/collection/immutable/List::dropWhile(Lscala/Function1;)Lscala/collection/immutable/List; contains the instruction INVOKESPECIAL scala/collection/immutable/List.loop$3 (Lscala/collection/immutable/List;Lscala/Function1;)Lscala/collection/immutable/List;
that would cause an IllegalAccessError when inlined into class Main$$anon$1.
List(0).dropWhile(_ < 1)
                 ^
one warning found

@scabug scabug mentioned this issue Apr 7, 2017
Closed
rtyley added a commit to guardian/frontend that referenced this issue Jul 12, 2022
@rtyley
Remove superfluous use of implicits.Collections
2d3c422 
In the course of upgrading the Frontend codebase to Scala 2.13 (see
#25190) we came across the
`implicits.Collections` class, and had trouble upgrading it:

#25190 (comment)

It turns out that we will be able to _delete_ this class with the Scala
2.13 upgrade, rather than have to update it, which is nice! To reduce
the size of the Scala 2.13 upgrade PR, we're removing superfluous use of
the `implicits.Collections` class in this pre-upgrade PR.

`implicits.Collections` is currently used to add two additional methods
to standard Scala collections:

* `distinctBy()` - introduced November 2012 with
  #263. Perhaps surprisingly,
  Scala 2.13 has a new built-in method that is called the same thing
  and is called the same way! So the Frontend implementation can be
  deleted when the Scala 2.13 upgrade occurs.
  https://www.scala-lang.org/api/2.13.x/scala/collection/Seq.html#distinctBy[B](f:A=%3EB):C

* `safeDropWhile()` - introduced January 2015 with
  #7706 to handle a problem
  with the Scala 2.11 compiler: scala/bug#7529
  The issue is no longer present in Scala 2.12, so the method could
  have been removed when the upgrade to Scala 2.12 was performed in
  November 2017 with #18218

This pre-upgrade PR deletes the unnecessary `safeDropWhile()` method,
and removes several unnecessary references to `implicits.Collections`.
@rtyley rtyley mentioned this issue Jul 12, 2022
Merged
rtyley added a commit to guardian/frontend that referenced this issue Jul 13, 2022
@rtyley
Remove superfluous use of implicits.Collections
edb3204 
In the course of upgrading the Frontend codebase to Scala 2.13 (see
#25190) we came across the
`implicits.Collections` class, and had trouble upgrading it:

#25190 (comment)

It turns out that we will be able to _delete_ this class with the Scala
2.13 upgrade, rather than have to update it, which is nice! To reduce
the size of the Scala 2.13 upgrade PR, we're removing superfluous use of
the `implicits.Collections` class in this pre-upgrade PR.

`implicits.Collections` is currently used to add two additional methods
to standard Scala collections:

* `distinctBy()` - introduced November 2012 with
  #263. Perhaps surprisingly,
  Scala 2.13 has a new built-in method that is called the same thing
  and is called the same way! So the Frontend implementation can be
  deleted when the Scala 2.13 upgrade occurs.
  https://www.scala-lang.org/api/2.13.x/scala/collection/Seq.html#distinctBy[B](f:A=%3EB):C

* `safeDropWhile()` - introduced January 2015 with
  #7706 to handle a problem
  with the Scala 2.11 compiler: scala/bug#7529
  The issue is no longer present in Scala 2.12, so the method could
  have been removed when the upgrade to Scala 2.12 was performed in
  November 2017 with #18218

This pre-upgrade PR deletes the unnecessary `safeDropWhile()` method,
and removes several unnecessary references to `implicits.Collections`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
optimizer
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@scabug