Tailrec patrol #7876

retronym · 2019-03-15T00:48:02Z

mark methods that are tailrec as such
make some more methods tailrec by making them final

- mark methods that are tailrec as such - make some more methods tailrec by making them final

retronym · 2019-03-15T00:50:18Z

src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala

        def loop(cases: List[CaseDef]): Option[CaseDef] = cases match {
          case head :: next :: _ if isDefault(head)                                    => Some(next) // subsumed by the next case, but faster
-          case head :: rest if !isGuardedCase(head) || head.guard.tpe =:= ConstantTrue => rest find caseImplies(head) orElse loop(rest)
+          case head :: rest if !isGuardedCase(head) || head.guard.tpe =:= ConstantTrue => rest find caseImplies(head) match { case s @ Some(_) => s case None => loop(rest) }


Hand inlined orElse to enable tail call elimination.

I'm especially impressed by the hardened resistance to semicolon.

retronym · 2019-03-15T00:50:58Z

src/compiler/scala/tools/nsc/typechecker/Contexts.scala

-      if (p(this)) this else outer.nextEnclosing(p)
+    @tailrec
+    final def nextEnclosing(p: Context => Boolean): Context =
+      if (this eq NoContext) this else if (p(this)) this else outer.nextEnclosing(p)


Removed override in NoContext to make this self-contained and final.

retronym · 2019-03-15T00:54:48Z

src/reflect/scala/reflect/internal/Symbols.scala


-    def isStrictFP: Boolean    = !isDeferred && (hasAnnotation(ScalaStrictFPAttr) || originalOwner.isStrictFP)
+    @tailrec
+    final def isStrictFP: Boolean    = this != NoSymbol && !isDeferred && (hasAnnotation(ScalaStrictFPAttr) || originalOwner.isStrictFP)


Removed override in NoSymbol to make this self-contained and final.

(I meant about the change in receiver.)

retronym · 2019-03-15T00:55:14Z

src/reflect/scala/reflect/internal/Symbols.scala

+      if (this eq NoSymbol) this
+      else if (isTopLevel) {
        if (isClass) this else moduleClass
      } else owner.enclosingTopLevelClass


Removed override in NoSymbol to make this self-contained and final.

retronym · 2019-03-15T00:55:26Z

src/reflect/scala/reflect/internal/Symbols.scala

+      if (this eq NoSymbol) this
+      else if (isTopLevel) {
        if (isClass) this else moduleClass.orElse(this)
      } else originalOwner.originalEnclosingTopLevelClassOrDummy


Removed override in NoSymbol to make this self-contained and final.

retronym · 2019-03-15T00:58:15Z

src/compiler/scala/tools/nsc/ast/DocComments.scala

+        case s @ Some(str)                   => s
+        case None                            => lookupVariable(vble, site.owner)
      }
  }


hand inlined orElse to enable tail call elimination

case s @ Some(_) avoids unused var. Or case s @ Some(str @ _) for baroque effect.

Recursion didn't come up in the discussions about "avoid match on options".

retronym · 2019-03-15T01:27:17Z

rollback the unintended changes to imports that IntelliJ made.

som-snytt · 2019-03-15T04:32:03Z

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

        checkHeadAssoc(leftAssoc)

+      @tailrec
      def loop(top: Tree): Tree = if (canReduce) {


Would be nice if -Xlint:loop enabled tailrec warning on anything called loop.

som-snytt · 2019-03-15T04:51:43Z

The current news in the airline industry focusing on automation and pilot training has me thinking about automation in software production.

som-snytt · 2019-03-15T05:03:23Z

The extra vertical space makes me wonder if tailrec should be assumed and for the rare case when I prefer to burn a stack frame, I can add @tailwreck.

It could be emitted as either warning or error, like deprecated and deprecatedError, and similarly be handled in the reporter (suppressed or escalated).

diesalbla · 2019-03-15T14:34:09Z

src/compiler/scala/tools/nsc/Global.scala


    /** Reset package class to state at typer (not sure what this is needed for?)
     */
+    @tailrec


Since we are at it, can we make this private[this]. In fact, can we backport #7127 to 2.12.x?

Someone could volunteer for private this patrol. Also, if they're not going to make private mean private this in scala 3, then -Xlint:private should make recommendations and -Xlint-fix should scalafix them.

I'd like to keep this PR focussed on the one cross-cutting improvement.

In general, 2.12.x development should start winding down, so backports need to pay their way

diesalbla · 2019-03-15T15:47:23Z

src/compiler/scala/tools/nsc/ast/TreeBrowsers.scala

    def format(width: Int, writer: Writer): Unit = {
      type FmtState = (Int, Boolean, Document)

+      @tailrec


BTW, would the pattern case List() two lines below be more efficient if it were case Nil?

or case _: Nil.type. Where is the empty list posse?

TIL UnapplySeqWrapper. Not sure whether to say "mind blown" or "code blown". Maybe this is why blandishments about List() are dangerous.

diesalbla · 2019-03-15T16:23:47Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

        // scala/bug#3971 unwrapping to the outermost Apply helps prevent confusion with the
        // error message point.
        def callee = {
+          @tailrec


Can we make the callee into a val or a lazy val? It looks to be called twice in the issueError, which is called from the match below.

som-snytt · 2019-03-16T20:04:16Z

Responding to my previous comment, Paul tried -Ytailrecommend for a few months a decade ago. A quick trial of -Xlint:tailrec needs some tweaks, and then suppressions will be needed for various weirdness, such as a map that delegates to super[SortedSet].map with a comment, "required for disambiguation". Either @tailwreck or reporter-based de-escalation.

reverts a small part of scala#7876 caught by community build

SethTisue · 2019-03-27T13:20:56Z

#7936 reverts a small piece of this. it's unfortunate that not all non-final classes that are extension points for scala-parallel-collections have comments marking them as such.

retronym added 3 commits March 15, 2019 10:46

Tailrec patrol: src/reflect

9efd913

- mark methods that are tailrec as such - make some more methods tailrec by making them final

Tailrec patrol: src/compiler

ca68c51

- mark methods that are tailrec as such - make some more methods tailrec by making them final

Tailrec patrol: src/library

a91d483

- mark methods that are tailrec as such - make some more methods tailrec by making them final

scala-jenkins added this to the 2.13.1 milestone Mar 15, 2019

retronym commented Mar 15, 2019

View reviewed changes

retronym force-pushed the topic/tailrec-patrol branch from ab8fadc to 803f4d0 Compare March 15, 2019 00:56

Revert unintented formatting change

8aae5b2

retronym force-pushed the topic/tailrec-patrol branch from 803f4d0 to 8aae5b2 Compare March 15, 2019 00:57

retronym commented Mar 15, 2019

View reviewed changes

retronym mentioned this pull request Mar 15, 2019

Add inner tail-recursive definitions #7720

Closed

Revert unwanted changes to imports

2849a0a

som-snytt reviewed Mar 15, 2019

View reviewed changes

som-snytt approved these changes Mar 15, 2019

View reviewed changes

diesalbla added the performance the need for speed. usually compiler performance, sometimes runtime performance. label Mar 15, 2019

diesalbla reviewed Mar 15, 2019

View reviewed changes

diesalbla approved these changes Mar 15, 2019

View reviewed changes

retronym merged commit cb47197 into scala:2.13.x Mar 20, 2019

retronym modified the milestones: 2.13.1, 2.13.0-RC1 Mar 20, 2019

SethTisue added a commit to SethTisue/scala that referenced this pull request Mar 27, 2019

keep TrieMapIterator non-final for parallel collections

792580b

reverts a small part of scala#7876 caught by community build

SethTisue mentioned this pull request Mar 27, 2019

keep TrieMapIterator non-final for parallel collections #7936

Merged

Tailrec patrol #7876

Tailrec patrol #7876

Uh oh!

Conversation

retronym commented Mar 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

retronym Mar 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

retronym commented Mar 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

som-snytt Mar 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

som-snytt commented Mar 15, 2019

Uh oh!

som-snytt commented Mar 15, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

som-snytt commented Mar 16, 2019

Uh oh!

SethTisue commented Mar 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

retronym commented Mar 15, 2019 •

edited

Loading

retronym Mar 15, 2019 •

edited

Loading

retronym commented Mar 15, 2019 •

edited

Loading

som-snytt Mar 15, 2019 •

edited

Loading