@lrytz thx for bumping this on the one-year anniversary.

🎈

Here's an example that shows the issue with this approach:

class A {
  def /: (z: A)(op: Int): A = ???
  def self = this
  def bar(x: A, y: A) = (x.self /: y.self)(1)
}

in typedApply of y.self./:(x.self), we build a block

{
  final <synthetic> <artifact> val rassoc$1: A = x.self;
  y.self./:(rassoc$1)
}

Calling typed(blk) fails because the typer through typedBlock, and typing blocks is not FUN in terms of mode. So we get

error: missing argument list for method /: in class A
Unapplied methods are only converted to functions when a function type is expected.

Just putting the typer in FUNmode wouldn't be enough, as the second param list needs to go into the block.

Names and defaults has the same problem, and does it with a lot of complexity (it builds a block for the inner Apply, then takes it apart for outer Applys and inserts them into the block). Here, it should be possible to avoid this complexity and just build the block at the very end, after type checking the full application (y.self./:(x.self)(1))?

A different issue caused by this PR is that we get a lint warning on (1, 2) :: Nil

Test.scala:2: warning: adapted the argument list to the expected 2-tuple: add additional parens instead
        signature: List.::[B >: A](elem: B): List[B]
  given arguments: 1, 2
 after adaptation: List.::((1, 2): (Int, Int))
  val x = (1,2) :: Nil
                ^

before the parser would spit out

    val x = {
      final <synthetic> <artifact> val rassoc$1 = scala.Tuple2(1, 2);
      Nil.$colon$colon(rassoc$1)
    }

but now it's val x = Nil.$colon$colon(1, 2).

Thanks @lrytz for the insight. If I wake up later today, I will try to grok it.

I wasn't sure if returning to this PR was worth the communal trouble; should I change the title to "Complexify right-associative rewrite"?

If we had mini-phases, we could have a mini-phase called p5073 after the ticket. Every ticket could have a test and a custom transform.

There's a similar challenge in the stabilization PR about whether to muck with trees in-flight, though there the effect is artificial rather than actual order of evaluation.

worth the communal trouble

I like the the simplification, it seems more tractable

similar challenge in the stabilization PR

Yeah.. I'm looking at scala/bug#11756 tomorrow, I think it would be good to have in 2.13.2, maybe just a minimal fix without changing the strategy.

@lrytz A year later, I piggybacked on your fix for stabilization.

Edit: I see the (42, 27) :: Nil idiom is idiomatic, so I will try to mitigate that. I mentioned it on discord.

Appselmode sounds like a Swiss tart made with little apples.

The previous easy tweak worked, but now:

[error] /home/travis/build/scala/scala/src/reflect/scala/reflect/internal/util/package.scala:17:23: Unused import
[error] import scala.language.existentials // scala/bug#6541
[error]                       ^
[error] one error found

I'll try a local bootstrap after all.

...oh I guess it's fatal on CI now. More late pandemic brain fog.

Except for the part where

[error] /home/jenkins/workspace/scala-2.13.x-validate-main/src/reflect/scala/reflect/internal/util/package.scala:46:13: inferred existential type Class[?0] forSome { type ?0 >: _$1; type _$1 }, which cannot be expressed by wildcards, should be enabled
[error] by making the implicit value scala.language.existentials visible.
[error] ----
[error] This can be achieved by adding the import clause 'import scala.language.existentials'
[error] or by setting the compiler option -language:existentials.
[error] See the Scaladoc for value scala.language.existentials for a discussion
[error] why the feature should be explicitly enabled.
[error]       clazz.getSuperclass :: clazz.getInterfaces.toList map (c => shortClass(c)) mkString " with "
[error]             ^
[error] one error found
[error] (reflect / Compile / compileIncremental) Compilation failed

Rebroke this:

    def shortClass(clazz: Class[_]): String = {
  final <synthetic> <artifact> val rassoc$1: Class[?0] forSome { type ?0 >: _$1; type _$1 } = clazz.getSuperclass();
  scala.Predef.wrapRefArray[Class[_]](clazz.getInterfaces()).toList.::[Class[_]](rassoc$1)
}.map[String](((c: Class[_]) => C.this.shortClass(c))).mkString(" with ")
  }

// vs
    def shortClass(clazz: Class[_]): String = {
      final <synthetic> <artifact> val rassoc$1: Class[?0] = clazz.getSuperclass();
      scala.Predef.wrapRefArray[Class[_]](clazz.getInterfaces()).toList.::[Class[_]](rassoc$1).map[String](((c: Class[_]) => C.this.shortClass(c))).mkString(" with ")
    }

Reviewing previous comments, I see the initial effort in 2019 needed APPSELmode; then the follow-up in 2021 prompted lrytz to ask did you even read my PR description, the way a professor might say did you even read the paper that describes this fundamental concept; not sure what I was talking about in 2022-23.

Currently, there are a few old TODO comments I wouldn't mind addressing that are out of scope;

• not sure the "arg" annotation is needed to detect that a view was applied (replacing the ApplyView with an attachment is one of the TODOs);
• test that deprecated infix type arg is not broken.
• The commit could be split into "trivial refactor or picking whitespace nits" and actual changes.
• 30 LOC transformRassoc could be moved to an outer scope where it is less distracting to the reader.
• run/t5073 is run/1980b.

Tip: You cannot create task list items within closed issues or issues with linked pull requests.

Does anyone get the Merriam-Webster email? Today's word is "steadfast". Their quiz:

Rearrange the letters to form a verb meaning "to make steadfast": ZBIILEAST

Rebased and followed up on the items. Did not split the first commit. Did remove the two special Apply classes, as handling them specially is error-prone; using attachments is comfortable by now.

Bug found while trying stuff

object T extends App {
  def x = println("x")
  def y = println("y")
  object Nol {
    def ::(x: Any)(implicit o: Ordering[Int]): Nol.type = Nol
  }

  x :: y :: Nol
}

The rassoc transformation is not done (not in your version, and not with my commit on top).

OK, I'm awake. Thanks for the code comments, which I read, it's useful to have a published paper that explains the theoretical basis.

I see why you didn't like the "special case" (I don't think you liked it two years ago). I'll have a second coffee before applying myself to review.

My first thought was that the new attachment is a different way to pronounce "tomato", but it's probably also a cleaner basis for fixing bugs in the general approach.

What would really help: a single test file for everything around stabilizers and rassocs, their combinations, interaction with implicit params and conversions (and other features?).

Scala 3 seems to get it all right, at least the few examples I tried. Can we align with the way it's implemented there? I didn't take a look.