Change scheme to implement creator applications

[odersky]

Use synthetic apply methods instead of a fallback in Typer to implement
creator applications.

Todo:

• Check for shadowing of creator proxies
• Investigate problem in zio: made a separate issue Investigate problem in ZIO's promise #10808
• Change reference doc page

[sjrd]

Will that affect the bytecode in any way? (new objects, new defs, call sites?)

[sjrd]

More importantly: can this change the meaning of existing programs?

• Through accidental overrides of an existing apply method in a superclass/trait of the companion?
• Through a change in overload resolution if there are other apply methods in the companion?
• Through shadowing an apply method available via an implicit conversion/extension method?

All the previous discussions of this feature were based on the fact that it couldn't change the meaning of existing programs. Changing this premise invalidates all those discussions, including at the SIP meetings, where I recall that this was an important concern.

[odersky]

More importantly: can this change the meaning of existing programs?

Through accidental overrides of an existing apply method in a superclass/trait of the companion?

No. If an apply method exists (with any type, in either the class itself or in any parent), no creator proxy is generated.

Through a change in overload resolution if there are other apply methods in the companion?

No, since creator proxies methods have lower precedence than anything else (note that the previous condition still allows overloads: An apply could be defined in trait A, and a creator proxy of a different signature in trait B. Then we do get overloaded variants in a class that extends A and B.

Through shadowing an apply method available via an implicit conversion/extension method?

Yes. And there could be other priority inversions as well. For instance here is one I observed in catsEffect2, class `InstancesTests.
It contains a call

AsyncTests[T1, T2]

In the same package a monomorphic class AsyncTests was defined. In an outer import (of lower precedence) an object AsyncTests was defined with a polymorphic apply method. The new scheme created a companion AsyncTests with the class that shadowed the outer companion. The problem could be fixed by importing the object in a more deeply nested scope
so that it takes precedence over the package definition.

We should look into what we can do to avoid breakage like that. Here, we got a type error, but we mighty also resolve to a different method which could change behavior. One possibility would be to try both old and new resolution schemes and issue an error if they are different. We could do that under -source 3.0-migration.

[odersky]

Here are the breakages observed in the community build:

• Shadowing problems of the kind described above. Fixed by 6 additional imports or qualifiers in cats-effects-2 (1), cats-mtl (3), scala-parallel collections (2).
• One issue in zio where some type parameters were not detected due to a cyclic reference that is probably caused by the completion of a constructor proxy (this one needs follow up).
• A weird issue where scalacheck used a case class named apply and relied on two applys in a row being inserted, which is no longer the case for some reason (I count that as a bug fix).
• An issue in scas that I believe comes from a build that mixes several compiler versions in an illegal way.

I believe we should address the shadowing problems. We could demand that when resolving a constructor proxy identifier the same identifier does not already exist as a regular identifier in some outer scope. That would have caught each of the shadowing issues. We should also see what we can do to avoid the problem in zio.

Overall I am quite positive now. The new scheme is much safer than the previous one, and it looks compatible enough with Scala 2 to be workable.

[odersky]

The code so far can already be reviewed.

[odersky]

I verified that the shadowing check catches and diagnoses the observed errors in the community build.

[odersky]

From my point of view, this is ready to merge. We can figure out the problem observed in zio later. If someone still has the time to review this until tomorrow, this would be great. Otherwise we'll merge as is and deal with any possible improvements later.

[odersky]

The shapeless error was interesting. It looks like we wanted the creator proxy since there was an explicit
import of its companions class. But we resolved to the outer apply before, so the fallback never kicked in.
Which shows that the fallback scheme can also be misleading.. Now we see that there's a conflict since
we get the shadowing error.

[odersky]

Overall I am now quite convinced that the new scheme is better than the previous one, and that the previous one had hidden problems that would have only got worse with widespread adoption. So I am glad we can get it changed in time for 3.0.

[odersky]

test performance please

[dottybot]

performance test scheduled: 20 job(s) in queue, 1 running.

[dottybot]

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/10784/ to see the changes.

Benchmarks is based on merging with master (d7ecc34)

[SethTisue]

before, I could val lock = AnyRef(), now I can't?

I guess it's because AnyRef a is type alias? I see also that class C; type D = C; D() worked before but now doesn't.

is this considered desirable/acceptable?

[rjolly]

I concur. This is the cause of the failure in ScAS. I have opened an issue: #10862

[odersky]

Yes, that's intended. You don't get an apply for a type alias. I think this is what we want. To compare, if I write

type S = scala.collection.immutable.Seq

I still can't do S(1, 2, 3). I'd have to add

val S = scala.collection.immutable.Seq

For the new kind of apply methods, it's the same.

[SethTisue]

(I guess let's discuss further on #10862)