Skip to content
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

Add note about type mismatch in automatically inserted apply argument #20023

Merged
merged 4 commits into from
May 1, 2024

Conversation

mbovel
Copy link
Member

@mbovel mbovel commented Mar 26, 2024

Started during the last spree with @jan-pieter and @iusildra.

Fixes #19680.

@mbovel mbovel requested a review from odersky March 26, 2024 14:19
@mbovel
Copy link
Member Author

mbovel commented Mar 26, 2024

This not ready yet, there would be subtleties to take care off like correctly line-wrapping the error message, but I am already adding @odersky as a reviewer for a high-level review: is the approach reasonable?

@mbovel
Copy link
Member Author

mbovel commented Apr 25, 2024

The note is not always relevant, for example consider:

def Test = List(1,2)("hello")

Does that clarify anything to say that:

  | The required type comes from a parameter of the automatically
  | inserted `apply` method of `List[Int]`,
  | which is the type of `List.apply[Int]([1,2 : Int]*)`.

?

As a middle ground, I moved the message to -explain.

Co-Authored-By: Jan-Pieter van den Heuvel <[email protected]>
Co-Authored-By: Lucas Nouguier <[email protected]>
@mbovel
Copy link
Member Author

mbovel commented Apr 26, 2024

The pickling failure in "Applications.scala" is just due to an extra line break…

➜  ~/dotty git:(mb/19680) diff before-pickling.txt after-pickling.txt
17956,17957c17956,17957
<                                                                                             dotty.tools.dotc.reporting.Diagnostic
<                                                                                               .Error
---
>                                                                                             dotty.tools.dotc.reporting.
>                                                                                               Diagnostic.Error

Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement. Otherwise LGTM

compiler/src/dotty/tools/dotc/typer/Applications.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/dotc/typer/Applications.scala Outdated Show resolved Hide resolved
compiler/src/dotty/tools/dotc/typer/Applications.scala Outdated Show resolved Hide resolved
@odersky odersky assigned mbovel and unassigned odersky Apr 27, 2024
@mbovel
Copy link
Member Author

mbovel commented Apr 29, 2024

The pickling failure in "Applications.scala" is just due to an extra line break…

➜  ~/dotty git:(mb/19680) diff before-pickling.txt after-pickling.txt
17956,17957c17956,17957
<                                                                                             dotty.tools.dotc.reporting.Diagnostic
<                                                                                               .Error
---
>                                                                                             dotty.tools.dotc.reporting.
>                                                                                               Diagnostic.Error

I was able to reproduce (with scala3-compiler-bootstrapped / testOnly *BootstrappedOnlyCompilationTests -- *picklingWithCompiler), but not to minimize and find exactly where and why the line breaking mechanism differs before and after pickling.

What I tried

I tried this, but it compiles successfully with -Xprint-types -Ytest-pickler -Yprint-pos -Yprint-pos-syms:

package dotty.tools.dotc.reporting.Diagnostic
package Diagnostic

class Error

def adipiscing() =
  def elit() =
    def nunc() =
      def vel() =
        def sapien() =
          def congue() =
            def lacus() =
              def duis() =
                def quam() =
                  def eleifend() =
                    def ultricies() =
                      def morbi() =
                        def pellentesque() =
                          val dia: Any = ???
                          dia match
                            case _: String =>
                              dia match
                              case _: String =>
                                dia match
                                  case _: String => new Diagnostic.Error()
                                  case _ => dia
                              case _ => dia
                            case _ => dia
                        pellentesque()
                      morbi()
                    ultricies()
                  eleifend()
                quam()
              duis()
            lacus()
          congue()
        sapien()
      vel()
    nunc()
  elit()

@main def Test = adipiscing()

As a workaround, I just moved maybeAddInsertedApplyNote to the outer scope, which avoids the problem.

@mbovel mbovel requested a review from odersky April 29, 2024 10:22
@mbovel mbovel assigned odersky and unassigned mbovel Apr 29, 2024
@mbovel mbovel marked this pull request as ready for review April 30, 2024 12:46
@mbovel
Copy link
Member Author

mbovel commented May 1, 2024

@odersky: I removed the last line of the message, as discussed.

@odersky odersky merged commit 084ab1a into scala:main May 1, 2024
19 checks passed
@mbovel mbovel deleted the mb/19680 branch May 1, 2024 08:11
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
WojciechMazur added a commit that referenced this pull request Jul 6, 2024
…y argument" to LTS (#21090)

Backports #20023 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

strange error for missing using keyword in implicit argument
3 participants