Skip to content

Compile failure with right associative op and conversion #15939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
som-snytt opened this issue Aug 31, 2022 · 3 comments · Fixed by #15957
Closed

Compile failure with right associative op and conversion #15939

som-snytt opened this issue Aug 31, 2022 · 3 comments · Fixed by #15957
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc itype:enhancement
Milestone

Comments

@som-snytt
Copy link
Contributor

Compiler version

Scala compiler version 3.1.3 -- Copyright 2002-2022, LAMP/EPFL
Scala compiler version 3.2.1-RC1-bin-SNAPSHOT-git-456444b -- Copyright 2002-2022, LAMP/EPFL

Minimized code

import scala.language.implicitConversions

object Test extends App {
  class Foo {
    class Bar {
      override def toString() = "bar"
    }
    object Bar {
      implicit def fromString(a: String): Bar = { println("convert bar") ; new Bar }
    }

    def andThen_:(b: Bar) = { println("pre") ; println(s"use $b") ; println("post") }
    def andThenByName_:(b: => Bar) = { println("pre") ; println(s"use $b") ; println(s"use $b") ; println("post") }
  }

  def mkFoo: Foo = { println("foo") ; new Foo }
  def mkBarString: String = { println("bar"); "Bar" }

  mkBarString andThen_: mkFoo

  println()

  mkFoo.andThen_:(mkBarString)

  println()

  mkBarString andThenByName_: mkFoo

  println()

  mkFoo.andThenByName_:(mkBarString)
}

Output

-- [E007] Type Mismatch Error: test/files/run/t4225e.scala:20:2 --------------------------------------------------------------------------
20 |  mkBarString andThen_: mkFoo
   |  ^^^^^^^^^^^
   |  Found:    String
   |  Required: Nothing
   |
   | longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: test/files/run/t4225e.scala:24:18 -------------------------------------------------------------------------
24 |  mkFoo.andThen_:(mkBarString)
   |                  ^^^^^^^^^^^
   |                  Found:    String
   |                  Required: Nothing
   |
   | longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: test/files/run/t4225e.scala:28:2 --------------------------------------------------------------------------
28 |  mkBarString andThenByName_: mkFoo
   |  ^^^^^^^^^^^
   |  Found:    String
   |  Required: Nothing
   |
   | longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: test/files/run/t4225e.scala:32:24 -------------------------------------------------------------------------
32 |  mkFoo.andThenByName_:(mkBarString)
   |                        ^^^^^^^^^^^
   |                        Found:    String
   |                        Required: Nothing
   |
   | longer explanation available when compiling with `-explain`

From -Vprint:typer

    def mkBarString: String =
      {
        println("bar")
        "Bar"
      }
    {
      val b$1: <error Found:    String
Required: Nothing> = mkBarString
      Test.mkFoo.andThen_:(b$1)
    }
    println()
    Test.mkFoo.andThen_:(mkBarString)
    println()
    Test.mkFoo.andThenByName_:(mkBarString)
    println()
    Test.mkFoo.andThenByName_:(mkBarString)

Expectation

Successful compilation of Scala 2 test test/files/run/t4225e.scala

@som-snytt som-snytt added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Aug 31, 2022
@odersky
Copy link
Contributor

odersky commented Sep 2, 2022

It has nothing to do with right associativity. The following also fails:

mport scala.language.implicitConversions

object Test {
  class Foo {
    class Bar {
      override def toString() = "bar"
    }
    object Bar {
      implicit def fromString(a: String): Bar = { println("convert bar") ; new Bar }
    }

    def andThen(b: Bar): Unit = { println("pre") ; println(s"use $b") ; println("post") }
  }

  def mkFoo: Foo = ???
  def mkBarString: String = ???
  mkFoo.andThen(mkBarString)
}

However, if I change the def mkFoo to a val mkFoo it works.

What goes on here:

The compiler needs to find a type for mkFor.andThen. It's a function over Bar but what's the prefix? mkFoo.Bar is not a legal type. So it decides to approximate, and since Bar occurs contravariantly the approximation goes down to Nothing. I tried to turn that off and skolemize the prefix by hacking the code in asSeenfrom but then I get:

-- [E007] Type Mismatch Error: test.scala:17:16 --------------------------------
17 |  mkFoo.andThen(mkBarString)
   |                ^^^^^^^^^^^
   |                Found:    String
   |                Required: ?9.Bar
   |
   |                where:    ?9 is an unknown value of type Test.Foo
   |

I don't think there's anything that can be done here except improve the error diagnostics, but even that is a huge problem since the logic that causes this is very far away from where we assemble error messages.

@odersky
Copy link
Contributor

odersky commented Sep 2, 2022

I believe the conclusion is that implicit conversions are a bad idea and implicit conversions in nested classes inside other classes are an even worse idea. Maybe we should outlaw implicit conversions?

@odersky odersky added itype:enhancement area:reporting Error reporting including formatting, implicit suggestions, etc and removed stat:needs triage Every issue needs to have an "area" and "itype" label itype:bug labels Sep 2, 2022
@som-snytt
Copy link
Contributor Author

som-snytt commented Sep 2, 2022

Indeed, the test is also to show order of operations when conversions are involved. Lukas's observation was that there is no correct answer. But other implicit lookups besides conversions may be required; I'll check those use cases.

smarter added a commit that referenced this issue Sep 4, 2022
The previous scheme did not set the approximated flag in some cases, which led
to the appearance of unhelpful Nothing types in error messages.

We now always mark the AsSeenFrom map as approximated when encountering an illegal prefix at variance <= 0.
But we reset it again when a range in a prefix is dropped because we can follow a type alias or use a
singleton type info.

Furthermore, we use an `Int`, `approxCount` instead of `approximated` to keep track of
multiple approximation points.

Fixes #15939 in the sense that the error message now makes more sense. We still cannot find the implicit conversion; that would require a more global measure to fix the problem (such as going to ANF) or existential types.
@Kordyjan Kordyjan added this to the 3.2.2 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc itype:enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants