fix override return type check

[jxnu-liguobin]

closed scala/bug#12621

[som-snytt]

I think it must be fixed in Namers where the overridden type is computed to use as expected type for RHS. RefChecks is not wrong.

[jxnu-liguobin]

Namers

Thank you, I'll take a look.

[SethTisue]

@jxnu-liguobin we've been slow with PR reviews lately, because of the 2.13.10 release and other factors... but I'm trying to get things moving again. mind rebasing this one to resolve the merge conflict? then we can try to find a reviewer.

[som-snytt]

The override inference behavior has needed a couple of passes for correctness. The conflict is probably due to those edits. I'm not sure how this issue will interact. In particular, there is also #10198 (which may not stand scrutiny).

[SethTisue]

I pushed a rebase — the merge conflict seemed easy to resolve. Let's see what CI thinks.

[SethTisue]

CI likes it. @som-snytt can I interest you in serving as reviewer here?

[som-snytt]

I'm taking a look. I'd like to understand why the pt is already widened by ExistentialExtrapolation:

Widened lone occurrence of sb.type inside existential to upper bound: StringBuilder

The proposed fix is downstream from there.

Edit: something to do with whether the param type is stable. Probably that only matters if the result is a member type sb.T. Currently, this works in the example:

  val x: StringBuilder = new StringBuilder
  def f(sb: x.type): sb.type = sb

[som-snytt]

I submitted an alternative that attempts to infer or construct the correct pt more directly. That happens before the RHS is typechecked.

The approach in this PR is to keep the type of the RHS, but pt has already been used (so the expected type was StringBuilder instead of sb.type).

I'll leave this PR open until I understand the alternatives better.

[som-snytt]

overrideChain may be too much; compare computeOverridden. What is isAliasType guarding? A test would show it.

[som-snytt]

I tried the neg test (from the other PR), and this compiles! To review, the overridden type becomes an existential with StringBuilder as upper bound, which is why StringBuilder is the expected type, so the RHS typechecks. After the horses have escaped through the barn door, we fix the result type of the overloading f so it seems correct after that.

class C { def f(sb: StringBuilder): sb.type = sb }

// It's ok, if not specify a return type.
class D extends C { override def f(sb: StringBuilder) = new StringBuilder }

Now I see that keepWhenMissResultTypeForSingleton is keeping sb.type, I had misread that big expr to mean it was keeping the singleton type of the RHS if it conformed to the overridden type.

[SethTisue]

(the other PR being #10334)

[SethTisue]

@jxnu-liguobin thank you for pursuing this as far as you did — looks like the baton is passed to Som

[som-snytt]

Also thanks for patience and actually taking the first two swings. It's a lot to chew on, and I might have had actionable feedback earlier if I knew what I was doing, or what the compiler is doing. I definitely would not have waded into these waters first. Also the other PR isn't over the finish line. It's not even on the last lap.