scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Project compiles both with Scala 3 and Scala 2, but not with -Xsource:3-cross #12975

Closed OndrejSpanel closed 8 months ago

OndrejSpanel commented 8 months ago

Reproduction steps

Scala version: 2.13.13

scalaVersion := "2.13.13"
[error] C:\Dev\ScalaSwingContrib\src\main\scala\scalaswingcontrib\tree\Tree.scala:77:43: type mismatch;
[error]  found   : info.type (with underlying type this.companion.CellInfo)
[error]  required: editor.companion.CellInfo
[error]         editor.componentFor(tree, toB(a), info)
[error]                                           ^
[error] C:\Dev\ScalaSwingContrib\src\main\scala\scalaswingcontrib\tree\Tree.scala:105:19: type mismatch;
[error]  found   : TreeEditors.this.Editor.CellInfo
[error]  required: Editor.this.companion.CellInfo
[error]           CellInfo(isSelected = selected, isExpanded = expanded, isLeaf = leaf, row = rowIndex)).peer
[error]                   ^
[error] C:\Dev\ScalaSwingContrib\src\main\scala\scalaswingcontrib\tree\Tree.scala:175:46: type mismatch;
[error]  found   : TreeRenderers.this.Renderer.CellInfo
[error]  required: Renderer.this.companion.CellInfo
[error]         componentFor(treeWrapper, a, CellInfo(isSelected=selected, isExpanded=expanded, isLeaf=leaf, row=rowIndex, hasFocus=focus)).peer
[error]                                              ^
[error] three errors found

Problem

I think it is strange and unexpected. I hoped -Xsource:3-cross will improve cross-building, not worsen it. I have similar problem in my closed source project, where I am unable to use -Xsource:3-cross because of this.

It would probably be possible to extract much more reduced repro, but I will do it only if the issue is considered interesting enough to be worth the effort.

som-snytt commented 8 months ago

Thanks, I'll take a look. Lukas improved the -Xsource:3-cross DX for 2.13.14 by letting you opt-in or opt-out of features which don't cross-compile nicely. I'm guessing this example might be due to "override inference".

OndrejSpanel commented 8 months ago

Good to know, I will be glad to check with 2.13.14 once able.

I'm guessing this example might be due to "override inference".

I expected something in this area, but I though those "cross" features should make Scala 2.13 to behave more like Scala 3. The project already compiles with Scala 3, which was quite non-trivial effort, the path dependent types in the Tree wrapper were a tough nut to crack.

SethTisue commented 8 months ago

@OndrejSpanel just to check, was your previous Scala version 2.13.12, or something older?

OndrejSpanel commented 8 months ago

For ScalaSwingContrib It was 2.13.12 in the most recent published version of the library.

crossScalaVersions := Seq("2.12.18", "2.13.12", "3.3.1")

The change in the repro steps in SBT is made from 3.3.1, so that the repro steps do not require ++ for Scala 2 compilation.

som-snytt commented 8 months ago

it requires 2.13.13 for -Xsource:3-cross.

Specifying a type for companion only fixes the 3rd error.

-    final val companion = Editor
+    final val companion: CellEditorCompanion = Editor

now in CellEditor

def componentFor(owner: Owner, value: A, cellInfo: companion.CellInfo): Component

where CellInfo is an abstract type member.

I'm not sure on what basis it's allowed (where it compiles) for Editor[A] to forward this call to Editor[B]:

      override def componentFor(tree: Tree[_], a: A, info: companion.CellInfo): Component = {
        editor.componentFor(tree, toB(a), info)
      }

oh I guess it's because companion is final. Any two Editor subclasses will have the same type for companion. Inference must lose that under 3-cross.

I remember scala-swing was one of the first things I looked at as a Scala learner and it was so clever.

OndrejSpanel commented 8 months ago

This Tree component is clever on steroids, a bit over-clever perhaps, but it works and nobody is interested in it enough to rework it significantly. I was glad when I have finished the Scala 3 port, I was already considering a raw Java tree component as a fallback and that was not a nice prospect. 😔

som-snytt commented 8 months ago

Under -Xsource:3 on 2.13.14

[error] .../ScalaSwingContrib/src/main/scala/scalaswingcontrib/tree/Tree.scala:161:15: in Scala 3 (or with -Xsource-features:infer-override), the inferred type changes to TreeRenderers.this.CellRendererCompanion instead of TreeRenderers.this.Renderer.type [quickfixable]
[error] Scala 3 migration messages are issued as errors under -Xsource:3. Use -Wconf or @nowarn to demote them to warnings or suppress.
[error] Applicable -Wconf / @nowarn filters for this fatal warning: msg=<part of the message>, cat=scala3-migration, site=scalaswingcontrib.tree.TreeRenderers.Renderer.companion
[error]     final val companion = Renderer
[error]               ^

The output is very hard to read because sbt [error] makes it hard to see where a message begins, and the extra lines of text plus the prefix make it hard to pick out the actual warning.

OndrejSpanel commented 8 months ago

With this the project compiles in 2.12, 2,13 and 3.3:

final val companion: Renderer.type = Renderer

With this it does not compile in any of them:

final val companion: CellRendererCompanion = Renderer

What is strange then is that with following it does compile in 3.3:

final val companion = Renderer

I though the new type overriding type rules make this to be the same as : CellRendererCompanion, as this is the type specified in the CellRenderer base class. Maybe it is because it is val, not method?

https://docs.scala-lang.org/scala3/guides/migration/incompat-type-inference.html

In Scala 3 the return type of an override method is inferred by inheritance from the base method, whereas in Scala 2.13 it is inferred from the left hand side of the override method

som-snytt commented 8 months ago

Yes, Scala 3 actually infers an intersection. I haven't looked at it in a long time, but intended to improve what Scala 2 does.

Note to self, consider improving output by using shorter lines, and putting "extra text" after the caret line.

image

OndrejSpanel commented 8 months ago

I confirm with both:

final val companion: Editor.type = Editor

final val companion: Renderer.type = Renderer

the project compiles even with -Xsource:3-cross.

I am not sure what is the lesson from this. I think -Xsource:3-cross is wrong (overzealous) and adjusts the type even where Scala 3 would not, but I am not sure what is the best action to be taken (if any).

som-snytt commented 8 months ago

I'll try to figure something out, or, more likely, Lukas will. Thanks for the report, and I'm glad at least you are not blocked for now.

som-snytt commented 8 months ago

No rockets yet, we forgot to test under 3.4, where

[error] -- [E007] Type Mismatch Error: .../ScalaSwingContrib/src/main/scala/scalaswingcontrib/tree/Tree.scala:77:42
[error] 77 |        editor.componentFor(tree, toB(a), info)
[error]    |                                          ^^^^
[error]    |                                   Found:    (info : companion.CellInfo)
[error]    |                                   Required: editor.companion.CellInfo
[error]    |
[error]    | longer explanation available when compiling with `-explain`
[error] -- [E007] Type Mismatch Error: .../ScalaSwingContrib/src/main/scala/scalaswingcontrib/tree/Tree.scala:106:18
[error] 106 |          CellInfo(isSelected = selected, isExpanded = expanded, isLeaf = leaf, row = rowIndex)).peer
[error]     |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]     |          Found:    TreeEditors.this.Editor.CellInfo
[error]     |          Required: Editor.this.companion.CellInfo²
[error]     |
[error]     |          where:    CellInfo  is a class in object Editor
[error]     |                    CellInfo² is a type in trait CellEditorCompanion
[error]     |
[error]     | longer explanation available when compiling with `-explain`

Edit: somehow, I can't reproduce this build error.

OndrejSpanel commented 8 months ago

Thanks. It was not much a personal blocker, I hope to be able to drop Scala 2 in my project soon, but it will be nice to be able to use some Scala 3 syntax even in the transition period, which Xsource:3-cross should allow.

I find an advice about Xsource:3-cross not to be used for projects which intend to migrate soon to Scala 3 strange, though, as the fact some Scala 3 semantics is present in Scala 2 makes it extremely useful: I do not have to use explicit type annotations in situations where I know Scala 3 rules will suit me well, while Xsource:3 forces me to insert explicit types everywhere.

No rockets yet, we forgot to test under 3.4, where

Good idea. I am testing with 3.4.1 now (with explicit types added as requested by Xsource:3) and it compiles to me just fine, with some deprecation warnings (mostly infix methods).

I have updated the scala3-maintenance branch with all my changes.

lrytz commented 8 months ago

I'll create a new ticket with a minimization (https://github.com/scala/bug/issues/12980).