Allow @implicitNotFound messages as explanations #16893

odersky · 2023-02-12T12:02:40Z

A problem of the @implicitNotFOund mechanism so far was that the user defined message replaced the compiler-generated one, which might lose valuable information.

This commit adds an alternative where an @implicitNotFound message that starts with ... is taken as an explanation (without the ...) enabled under -explain. The compiler-generated message is then kept as the explicit error message.

We apply the mechanism for an @implicitNotFound message for boundary.Label. This now produces messages like this one:

-- [E172] Type Error: tests/neg-custom-args/explain/labelNotFound.scala:2:30 -------------------------------------------
2 |  scala.util.boundary.break(1) // error
  |                              ^
  |No given instance of type scala.util.boundary.Label[Int] was found for parameter label of method break in object boundary
  |---------------------------------------------------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  | A Label is generated from an enclosing `scala.util.boundary` call.
  | Maybe that boundary is missing?
   ---------------------------------------------------------------------------------------------------------------------

A problem of the @implicitNotFOund mechanism so far was that the user defined message replaced the compiler-generated one, which might lose valuable information. This commit adds an alternative where an @implicitNotFound message that starts with `...` is taken as an explanation (without the ...) enabled under -explain. The compiler-generated message is then kept as the explicit error message. We apply the mechanism for an @implicitNotFound message for `boundary.Label`. This now produces messages like this one: ``` -- [E172] Type Error: tests/neg-custom-args/explain/labelNotFound.scala:2:30 ------------------------------------------- 2 | scala.util.boundary.break(1) // error | ^ |No given instance of type scala.util.boundary.Label[Int] was found for parameter label of method break in object boundary |--------------------------------------------------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | A Label is generated from an enclosing `scala.util.boundary` call. | Maybe that boundary is missing? --------------------------------------------------------------------------------------------------------------------- ```

odersky · 2023-02-12T12:04:33Z

@nicolasstucki We can use an analogous scheme to improve the situation for #16888.

prolativ · 2023-02-14T14:42:08Z

Should we somehow document this new behaviour or is it supposed to be used only internally?

nicolasstucki · 2023-02-14T15:58:40Z

Unfortunately, the spec of implicitNotFound states how the error will be shown. Changing this could break existing use cases.

A solution to this might be to show the custom message and hide the compiler-generated message under -explain.

The message A Label is generated from an enclosing `scala.util.boundary` call. Maybe that boundary is missing? does not seem to fit exactly the intended use of implicitNotFound. It feels like we should have a way to customize the message and the -explain. The example would be the custom -explain. We could modify the class implicitNotFound to take a message and explain string or add a separate @implicitNotFoundExplain.

Maybe this is how an error message should look

-- [E172] Type Error: tests/neg-custom-args/explain/labelNotFound.scala:2:30 -------------------------------------------
2 |  scala.util.boundary.break(1) // error
  |                              ^
  |  missing Int boundary label
  |---------------------------------------------------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  | No given instance of type scala.util.boundary.Label[Int] was found for parameter label of method break in object boundary
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  | A Label[Int] is generated from an enclosing `scala.util.boundary` call.
  | Maybe that boundary is missing?
  | Maybe a given Label[Int] is missing from the parameters?
   ---------------------------------------------------------------------------------------------------------------------

nicolasstucki

We should not break the contract of implicitNotFound

odersky · 2023-02-14T16:57:39Z

Unfortunately, the spec of implicitNotFound states how the error will be shown. Changing this could break existing use cases.

In theory yes, in practice, I would be really surprised if someone started an implicitNotFound message with ....

odersky · 2023-02-14T16:59:21Z

But I guess it's OK to add an implicitNoFoundExplain annotation. Do you want to add that one? I just wanted to show what needs to be done, but won't have the time to do it.

prolativ · 2023-02-16T09:36:45Z

Adding a new annotation would have to be done in a minor release, unless we make it experimental

bishabosha · 2023-02-16T09:44:24Z

Adding a new annotation would have to be done in a minor release, unless we make it experimental

the release candidates (e.g. 3.3.0-RC2) still have experimental tasty version, so its ok to release for 3.3.0?

odersky · 2023-02-16T09:48:49Z

So somebody should do it quickly! These are real and very important usability improvements. We should not block them by procedural reasons.

prolativ · 2023-02-16T12:48:03Z

How would we like this new annotation to be used then? Should it be orthogonal to implicitNotFound, meaning that the old annotation would only replace the error message and the new one would only add explanation?

odersky · 2023-02-16T16:26:13Z

How would we like this new annotation to be used then? Should it be orthogonal to implicitNotFound, meaning that the old annotation would only replace the error message and the new one would only add explanation?

Yes, I think that's reasonable.

smarter · 2023-02-16T21:55:15Z

In Scala 2, @implicitNotFound does not replace the message, both are shown. This difference in behavior has lead to some confusion as described in #11493, and it seems people are happy with doing it the Scala 2 way which wouldn't require introducing a new annotation.

odersky · 2023-02-17T09:08:37Z

@smarter I did not know that Scala 2 behaved differently. Yes, let's go back to that then the whole discussion about explain becomes redundant.

nicolasstucki · 2023-02-17T09:48:32Z

In Scala 2, @implicitNotFound does not replace the message, both are shown. This difference in behavior has lead to some confusion as described in #11493, and it seems people are happy with doing it the Scala 2 way which wouldn't require introducing a new annotation.

It does not seem to be the case. In Scala 2 only the custom message is shown. It seems that the behaviour is the same as in Scala 3.

@scala.annotation.implicitNotFound("Foo[${T}] was not found")
final class Foo[T]
class C {
  def test = implicitly[Foo[Int]]
}

~/GitHub/dotty @ main % scala-cli compile -S 2.13 Foo.scala
Compiling project (Scala 2.13.10, JVM)
[error] ./Foo.scala:4:14: Foo[Int] was not found
[error]   def test = implicitly[Foo[Int]]
[error]              ^^^^^^^^^^^^^^^^^^^^
Error compiling project (Scala 2.13.10, JVM)
Compilation failed
~/GitHub/dotty @ main % scala-cli compile -S 3.2 Foo.scala
Compiling project (Scala 3.2.2, JVM)
[error] ./Foo.scala:4:34: Foo[Int] was not found
[error]   def test = implicitly[Foo[Int]]
[error]                                  ^
Error compiling project (Scala 3.2.2, JVM)
Compilation failed

prolativ · 2023-02-17T12:01:48Z

It looks like the behaviour in scala 2 depends on whether the annotation is inherited or not.

import annotation.implicitNotFound

@implicitNotFound("Missing type: Foo")
trait Foo

@implicitNotFound("Missing type: Bar")
trait Bar

trait Baz extends Bar

object Test {
  def myImplicitly[T](implicit @implicitNotFound("Missing parameter: ${T}") t: T): T = t

  val a = implicitly[Int]
  val b = implicitly[Foo]
  val c = implicitly[Baz]

  val x = myImplicitly[Int]
  val y = myImplicitly[Foo]
  val z = myImplicitly[Baz]
}

Compiling with scala 2:

scala-cli compile Test.scala -S 2.13.10

[error] ./Test.scala:14:11: could not find implicit value for parameter e: Int
[error]   val a = implicitly[Int]
[error]           ^^^^^^^^^^^^^^^
[error] ./Test.scala:15:11: Missing type: Foo
[error]   val b = implicitly[Foo]
[error]           ^^^^^^^^^^^^^^^
[error] ./Test.scala:16:11: could not find implicit value for parameter e: Baz (Missing type: Bar)
[error]   val c = implicitly[Baz]
[error]           ^^^^^^^^^^^^^^^
[error] ./Test.scala:18:11: Missing parameter: Int
[error]   val x = myImplicitly[Int]
[error]           ^^^^^^^^^^^^^^^^^
[error] ./Test.scala:19:11: Missing parameter: Foo
[error]   val y = myImplicitly[Foo]
[error]           ^^^^^^^^^^^^^^^^^
[error] ./Test.scala:20:11: Missing parameter: Baz
[error]   val z = myImplicitly[Baz]
[error]           ^^^^^^^^^^^^^^^^^

Compiling with scala 3:

scala-cli compile Test.scala -S 3.3.0-RC2

[error] ./Test.scala:14:26: No given instance of type Int was found for parameter e of method implicitly in object Predef
[error]   val a = implicitly[Int]
[error]                          ^
[error] ./Test.scala:15:26: Missing type: Foo
[error]   val b = implicitly[Foo]
[error]                          ^
[error] ./Test.scala:16:26: Missing type: Bar
[error]   val c = implicitly[Baz]
[error]                          ^
[error] ./Test.scala:18:28: Missing parameter: Int
[error]   val x = myImplicitly[Int]
[error]                            ^
[error] ./Test.scala:19:28: Missing parameter: Foo
[error]   val y = myImplicitly[Foo]
[error]                            ^
[error] ./Test.scala:20:28: Missing parameter: Baz
[error]   val z = myImplicitly[Baz]
[error]                            ^

odersky · 2023-02-17T15:28:46Z

I'd vote for showing both messages always. It seems the implicit not found messages are not always that specific, so adding info which implicit parameter was not found is useful.

nicolasstucki · 2023-02-17T15:42:17Z

I'd vote for showing both messages always. It seems the implicit not found messages are not always that specific, so adding info which implicit parameter was not found is useful.

Sounds good. But we should put the custom error first.

odersky · 2023-02-17T16:52:25Z

Sounds good. But we should put the custom error first.

I am not sure about that. Can we do play out some scenarios how it would look?

For the missing label we'd get with custom error last:

No given instance of type scala.util.boundary.Label[Int] was found for parameter label of method break in object boundary

A Label is generated from an enclosing `scala.util.boundary` call.
Maybe that boundary is missing?`

With custom error first this makes no sense.

Another use case is the implicitNotFound for <:< Here we would have with custom error first:

Cannot prove that Int <:< String.

No given instance of type <:<[Int, String] was found for parameter xs of method flatten.

Or, custom error last:

No given instance of type <:<[Int, String] was found for parameter xs of method flatten.

Cannot prove that Int <:< String.

Here it's debatable, but I think in the end I would also prefer custom error last.

prolativ · 2023-02-20T11:48:21Z

I believe we have two kinds of use cases:

The @implicitNotFound annotation is intended to hide the fact that some library uses implicits in it's API to make it more friendly to users who are not familiar with the concept of implicits, e.g.

package calendar

import annotation.implicitNotFound

trait DateFormat

object DateFormats {
  implicit object DayMonthYear extends DateFormat
  implicit object MonthDayYear extends DateFormat
}

class Date(day: Int, month: Int, year: Int) {
  def show(implicit @implicitNotFound("To show a date one needs to specify a date format, e.g. with `import calendar.DateFormats.DayMonthYear`") dateFormat: DateFormat): String = ???
}

In this situation it might make sense to:
a) show only the custom message by default
b) show the custom message followed by the generic No given instance... message and a link to the documentation describing what implicits/givens are, when -explain is enabled

The message from @implicitNotFound is intended to give some clue (to slightly more advanced users) why exactly a given instance was not found - in this case the custom message seems to make more sense when shown after the generic message

The problem is that it's not so easy to tell which situation we're dealing with in general

prolativ · 2023-02-20T11:55:20Z

In general it seems like @implicitNotFound is slightly controversial, e.g. typelevel/cats#4060

odersky · 2023-02-20T11:58:40Z

I believe the cats example shows that we want additional info, not a replacement of the error message. E.g. see this comment:

I think this annotation is still useful, if you want to provide additional context (e.g., how to get that implicit). That seems to have been the original goal in typelevel/cats#581, although it was never executed (?) and isn't relevant anymore anyway. But just saying what is missing is something the compiler does anyway, and I think (hope) has been doing for a while? :)

prolativ · 2023-02-20T12:35:20Z

Yeah, I would say cats classifies as case 2

odersky · 2023-02-20T12:50:40Z

And I would argue the DateFormat case also classifies as case 2, not 1. Taken alone, the implicitNotFound message is more confusing than helpful. If it is shown after the regular message it makes sense.

I think by now we have not seen a single case where custom first is clearly better and lots of cases where it makes not sense at all. Let's close this issue and implement it.

This approach mirrors the format used in `@noWarn`. There it is possible to tag encode the category and message in the string. ``` @nowarn("cat=deprecation&msg=Implementation ...") ``` The current implementation for `explain=` only allows this tag at the start of the string. Making the message either a old-style message or an message in the explain. We could extend this to support `msg=` to be able to have both a custom message and an explain. Furthermore we could insert some flags to explicitly state how to display the original message (for example: not-found=hidden, not-found=in-message, not-found=in-explain).

Fixes scala#16888

nicolasstucki · 2023-02-28T13:46:22Z

I ended up keeping the current scheme for generating -explain messages but modified the format slightly to make it clearer and easily extensible. See 7de74bd.

I also added the implicit not found explain for Quotes.

nicolasstucki · 2023-03-02T19:16:26Z

@sjrd could you review 7de74bd? The spec part of it.

sjrd · 2023-03-03T08:54:52Z

What do you call "the spec part of it"? The commit message? I see no spec changes in the PR diff.

nicolasstucki · 2023-03-05T08:16:34Z

I meant the addition of the explain= tag in the message and how we special case it.

nicolasstucki · 2023-03-05T08:18:41Z

Can I create a copy of ImplicitNotFound into the dotty library and update the documentation? Not sure if that could cause conflicts when lading the compiled binaries.

library/src/scala/quoted/Quotes.scala

odersky

Small typo, otherwise LGTM

sjrd · 2023-03-07T13:52:37Z

I meant the addition of the explain= tag in the message and how we special case it.

That seems fine to me.

tests/neg-custom-args/explain/i16888.check

nicolasstucki · 2023-03-09T10:22:51Z

I meant the addition of the explain= tag in the message and how we special case it.

That seems fine to me.

Good, then it is ready to be merged.

odersky requested a review from nicolasstucki February 12, 2023 12:03

odersky assigned nicolasstucki Feb 12, 2023

nicolasstucki assigned odersky and unassigned nicolasstucki Feb 14, 2023

nicolasstucki suggested changes Feb 14, 2023

View reviewed changes

Kordyjan assigned nicolasstucki and unassigned odersky Feb 27, 2023

nicolasstucki added 2 commits February 28, 2023 14:16

Helpful implicit not found message for Quotes

8210235

Fixes scala#16888

nicolasstucki approved these changes Mar 2, 2023

View reviewed changes

nicolasstucki requested a review from sjrd March 2, 2023 19:13

nicolasstucki assigned sjrd and unassigned nicolasstucki Mar 2, 2023

odersky commented Mar 7, 2023

View reviewed changes

library/src/scala/quoted/Quotes.scala Outdated Show resolved Hide resolved

odersky commented Mar 7, 2023

View reviewed changes

Update library/src/scala/quoted/Quotes.scala

ff99617

odersky commented Mar 7, 2023

View reviewed changes

tests/neg-custom-args/explain/i16888.check Outdated Show resolved Hide resolved

Update tests/neg-custom-args/explain/i16888.check

4b83f1f

nicolasstucki approved these changes Mar 9, 2023

View reviewed changes

nicolasstucki merged commit fd91ce1 into scala:main Mar 9, 2023

nicolasstucki deleted the add-implicitnotfound-explain branch March 9, 2023 10:23

nicolasstucki mentioned this pull request Mar 9, 2023

Helpful implicit not found message for quotes.Quotes #16914

Closed

Kordyjan added this to the 3.3.1 milestone Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow @implicitNotFound messages as explanations #16893

Allow @implicitNotFound messages as explanations #16893

odersky commented Feb 12, 2023

odersky commented Feb 12, 2023 •

edited

Loading

prolativ commented Feb 14, 2023

nicolasstucki commented Feb 14, 2023

nicolasstucki left a comment

odersky commented Feb 14, 2023

odersky commented Feb 14, 2023

prolativ commented Feb 16, 2023

bishabosha commented Feb 16, 2023 •

edited

Loading

odersky commented Feb 16, 2023

prolativ commented Feb 16, 2023

odersky commented Feb 16, 2023

smarter commented Feb 16, 2023

odersky commented Feb 17, 2023

nicolasstucki commented Feb 17, 2023

prolativ commented Feb 17, 2023

odersky commented Feb 17, 2023

nicolasstucki commented Feb 17, 2023

odersky commented Feb 17, 2023

prolativ commented Feb 20, 2023

prolativ commented Feb 20, 2023

odersky commented Feb 20, 2023

prolativ commented Feb 20, 2023

odersky commented Feb 20, 2023

nicolasstucki commented Feb 28, 2023 •

edited

Loading

nicolasstucki commented Mar 2, 2023

sjrd commented Mar 3, 2023

nicolasstucki commented Mar 5, 2023

nicolasstucki commented Mar 5, 2023

odersky left a comment

sjrd commented Mar 7, 2023

nicolasstucki commented Mar 9, 2023

Allow @implicitNotFound messages as explanations #16893

Allow @implicitNotFound messages as explanations #16893

Conversation

odersky commented Feb 12, 2023

odersky commented Feb 12, 2023 • edited Loading

prolativ commented Feb 14, 2023

nicolasstucki commented Feb 14, 2023

nicolasstucki left a comment

Choose a reason for hiding this comment

odersky commented Feb 14, 2023

odersky commented Feb 14, 2023

prolativ commented Feb 16, 2023

bishabosha commented Feb 16, 2023 • edited Loading

odersky commented Feb 16, 2023

prolativ commented Feb 16, 2023

odersky commented Feb 16, 2023

smarter commented Feb 16, 2023

odersky commented Feb 17, 2023

nicolasstucki commented Feb 17, 2023

prolativ commented Feb 17, 2023

odersky commented Feb 17, 2023

nicolasstucki commented Feb 17, 2023

odersky commented Feb 17, 2023

prolativ commented Feb 20, 2023

prolativ commented Feb 20, 2023

odersky commented Feb 20, 2023

prolativ commented Feb 20, 2023

odersky commented Feb 20, 2023

nicolasstucki commented Feb 28, 2023 • edited Loading

nicolasstucki commented Mar 2, 2023

sjrd commented Mar 3, 2023

nicolasstucki commented Mar 5, 2023

nicolasstucki commented Mar 5, 2023

odersky left a comment

Choose a reason for hiding this comment

sjrd commented Mar 7, 2023

nicolasstucki commented Mar 9, 2023

odersky commented Feb 12, 2023 •

edited

Loading

bishabosha commented Feb 16, 2023 •

edited

Loading

nicolasstucki commented Feb 28, 2023 •

edited

Loading