Take into account the result type of inline implicit conversions unless they are transparent #17924

julienrf · 2023-06-06T13:54:40Z

In the first commit, I add a failing test:

import scala.language.implicitConversions

class Foo

object Foo:
  inline implicit def toFoo(x: Int): Foo = Foo()

object Usage:
  1.asdf // error

We expect that code to not compile but the error reported by the compiler is confusing as it suggests importing Foo.toFoo to resolve the compilation error. You can see this in the test report.

The problem comes from the fact that currently when the compiler checks whether an implicit conversion is applicable to an expression that fails to compile, it does not take into account the expected result type (here, ? { def asdf: ? }) if the candidate is an inline definition.

Instead, I believe the expected result type should be taken into account unless the candidate is a transparent inline. I make this change in the second commit, which makes the test pass.

Fixes #9685

julienrf · 2023-06-13T12:46:53Z

tests/neg-macros/i9685bis.check

+   |
+   |  The following import might make progress towards fixing the problem:
+   |
+   |    import foo.Baz.toBaz


Note that only the transparent inline conversion is suggested.

julienrf · 2023-06-13T12:47:19Z

compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala

     *  In that case, we should always succeed and not constrain type parameters in the expected type,
     *  because the actual return type can be a subtype of the currently known return type.
     *  However, we should constrain parameters of the declared return type. This distinction is
     *  achieved by replacing expected type parameters with wildcards.
     */
    def constrainResult(meth: Symbol, mt: Type, pt: Type)(using Context): Boolean =
-      if (Inlines.isInlineable(meth)) {
+      if (Inlines.isInlineable(meth) && meth.is(Transparent)) {


I am not 100% this is the right place to perform this test. Please let me know if there is a more appropriate place that contains more logic related to transparent definitions.

…ompiler Currently, the compiler incorrectly reports errors like: ~~~ text [error] val n: Int = 0; val s: Int = n + "hello" [error] ^ [error] Found: String [error] Required: Int [error] [error] One of the following imports might make progress towards fixing the problem: [error] [error] import org.specs2.io.FileName.ToFileName [error] import org.specs2.TimeoutSpecExample.asExecutionIsInterpolated [error] import org.specs2.reflect.FromObjectName.asExecutionIsInterpolated [error] import org.specs2.reporter.reporterSpecSupport.asExecutionIsInterpolated [error] import org.specs2.runner.RunnerSpecification.asExecutionIsInterpolated [error] import org.specs2.specification.S1.asExecutionIsInterpolated [error] import org.specs2.specification.S2.asExecutionIsInterpolated [error] import org.specs2.specification.dsl1.asExecutionIsInterpolated [error] import org.specs2.TimeoutSpecExample.asResultIsInterpolated [error] import org.specs2.reflect.FromObjectName.asResultIsInterpolated ~~~ But after scala/scala3#17924 is merged, the import suggestions will be removed from the output. We adjust the test specification in TypecheckSpec to pass both with the current compiler and with the fix in the linked PR.

…ompiler (#1165) Currently, the compiler incorrectly reports errors like: ~~~ text [error] val n: Int = 0; val s: Int = n + "hello" [error] ^ [error] Found: String [error] Required: Int [error] [error] One of the following imports might make progress towards fixing the problem: [error] [error] import org.specs2.io.FileName.ToFileName [error] import org.specs2.TimeoutSpecExample.asExecutionIsInterpolated [error] import org.specs2.reflect.FromObjectName.asExecutionIsInterpolated [error] import org.specs2.reporter.reporterSpecSupport.asExecutionIsInterpolated [error] import org.specs2.runner.RunnerSpecification.asExecutionIsInterpolated [error] import org.specs2.specification.S1.asExecutionIsInterpolated [error] import org.specs2.specification.S2.asExecutionIsInterpolated [error] import org.specs2.specification.dsl1.asExecutionIsInterpolated [error] import org.specs2.TimeoutSpecExample.asResultIsInterpolated [error] import org.specs2.reflect.FromObjectName.asResultIsInterpolated ~~~ But after scala/scala3#17924 is merged, the import suggestions will be removed from the output. We adjust the test specification in TypecheckSpec to pass both with the current compiler and with the fix in the linked PR.

…ompiler (etorreborre#1165) Currently, the compiler incorrectly reports errors like: ~~~ text [error] val n: Int = 0; val s: Int = n + "hello" [error] ^ [error] Found: String [error] Required: Int [error] [error] One of the following imports might make progress towards fixing the problem: [error] [error] import org.specs2.io.FileName.ToFileName [error] import org.specs2.TimeoutSpecExample.asExecutionIsInterpolated [error] import org.specs2.reflect.FromObjectName.asExecutionIsInterpolated [error] import org.specs2.reporter.reporterSpecSupport.asExecutionIsInterpolated [error] import org.specs2.runner.RunnerSpecification.asExecutionIsInterpolated [error] import org.specs2.specification.S1.asExecutionIsInterpolated [error] import org.specs2.specification.S2.asExecutionIsInterpolated [error] import org.specs2.specification.dsl1.asExecutionIsInterpolated [error] import org.specs2.TimeoutSpecExample.asResultIsInterpolated [error] import org.specs2.reflect.FromObjectName.asResultIsInterpolated ~~~ But after scala/scala3#17924 is merged, the import suggestions will be removed from the output. We adjust the test specification in TypecheckSpec to pass both with the current compiler and with the fix in the linked PR.

julienrf · 2023-06-14T13:59:38Z

The CI is green and the PR is ready for review.

nicolasstucki

@julienrf could you squash the commits before we merge this?

…ss they are transparent Fixes scala#9685

julienrf · 2023-06-16T13:42:23Z

Thank you for the review. I squashed the commits.

nicolasstucki · 2023-06-19T12:47:18Z

Did this work in 3.3.0? If not, we do not necessarily need to backport this and may wait until 3.3.2.

julienrf · 2023-06-19T13:03:52Z

No, the bug was present since 3.0.0.

We apply the full fix under `-source future` only, and a partial fix otherwise. This is a followup of scala#17924 that fixes the source incompatibilities reported in scala#18130. We test that the behavior under `-source future` still fixes scala#9685, and that without `-source future` we fix both scala#9685 and scala#18130.

We apply the full fix under `-source future` only, and a partial fix otherwise. This is a followup of scala#17924 that fixes the source incompatibilities reported in scala#18123. We test that the behavior under `-source future` still fixes scala#9685, and that without `-source future` we fix both scala#9685 and scala#18123.

Fixes #18123 Follow-up of #17924

Proposed in #19415 (comment) Fixes #19415 Reverts #18130 and #17924. #18130 is still closed as it was introduced by #17924. This reopens #9685

julienrf force-pushed the fix-confusing-import-suggestion branch from 72bfe2b to 1e7bf82 Compare June 13, 2023 11:10

julienrf marked this pull request as ready for review June 13, 2023 11:12

julienrf changed the title ~~Investigate issue #9685~~ Take into account the result type of inline implicit conversions unless they are transparent Jun 13, 2023

julienrf commented Jun 13, 2023

View reviewed changes

This comment was marked as resolved.

Sign in to view

julienrf requested a review from odersky June 13, 2023 13:38

This was referenced Jun 13, 2023

Don’t require a newline after a type mismatch error reported by the compiler etorreborre/specs2#1165

Merged

Remove import suggestions from assertions scalameta/munit#663

Draft

julienrf force-pushed the fix-confusing-import-suggestion branch from 0d7b97d to 67c6a55 Compare June 14, 2023 12:43

julienrf requested a review from nicolasstucki June 14, 2023 13:59

nicolasstucki approved these changes Jun 16, 2023

View reviewed changes

Take into account the result type of inline implicit conversions unle…

eb38e1f

…ss they are transparent Fixes scala#9685

julienrf force-pushed the fix-confusing-import-suggestion branch from 67c6a55 to eb38e1f Compare June 16, 2023 13:41

nicolasstucki approved these changes Jun 19, 2023

View reviewed changes

nicolasstucki merged commit eb84bdd into scala:main Jun 19, 2023
17 checks passed

julienrf deleted the fix-confusing-import-suggestion branch June 19, 2023 07:23

julienrf added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Jun 19, 2023

nicolasstucki added this to the 3.3.1 milestone Jun 19, 2023

julienrf mentioned this pull request Jun 19, 2023

Update munit in the community build #18005

Closed

julienrf removed the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Jun 19, 2023

julienrf modified the milestones: 3.3.1, 3.3.2 Jun 20, 2023

This was referenced Jul 4, 2023

Followup fix to transparent inline conversion #18130

Merged

Explicitly upcast intermediate result to aid program elaboration stryker-mutator/weapon-regex#230

Merged

bishabosha pushed a commit that referenced this pull request Jul 10, 2023

Followup fix to transparent inline conversion (#18130)

3de184c

Fixes #18123 Follow-up of #17924

Kordyjan modified the milestones: 3.3.2, 3.4.0 Aug 1, 2023

odersky mentioned this pull request Jan 27, 2024

Regression in kalin-rudnicki/harness #19415

Closed

nicolasstucki mentioned this pull request Mar 12, 2024

Revert broken changes with transparent inline #19922

Merged

nicolasstucki added a commit that referenced this pull request Mar 12, 2024

Revert broken changes with transparent inline (#19922)

f663a8b

Proposed in #19415 (comment) Fixes #19415 Reverts #18130 and #17924. #18130 is still closed as it was introduced by #17924. This reopens #9685

noti0na1 added a commit to dotty-staging/dotty that referenced this pull request May 14, 2024

Revert changes in CB to scala#17924

d29a4a8

odersky pushed a commit to dotty-staging/dotty that referenced this pull request Jun 11, 2024

Revert changes in CB to scala#17924

d8a3d3b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Take into account the result type of inline implicit conversions unless they are transparent #17924

Take into account the result type of inline implicit conversions unless they are transparent #17924

julienrf commented Jun 6, 2023 •

edited

Loading

julienrf Jun 13, 2023

julienrf Jun 13, 2023

This comment was marked as resolved.

julienrf commented Jun 14, 2023

nicolasstucki left a comment

julienrf commented Jun 16, 2023

nicolasstucki commented Jun 19, 2023

julienrf commented Jun 19, 2023

Take into account the result type of inline implicit conversions unless they are transparent #17924

Take into account the result type of inline implicit conversions unless they are transparent #17924

Conversation

julienrf commented Jun 6, 2023 • edited Loading

julienrf Jun 13, 2023

Choose a reason for hiding this comment

julienrf Jun 13, 2023

Choose a reason for hiding this comment

This comment was marked as resolved.

julienrf commented Jun 14, 2023

nicolasstucki left a comment

Choose a reason for hiding this comment

julienrf commented Jun 16, 2023

nicolasstucki commented Jun 19, 2023

julienrf commented Jun 19, 2023

julienrf commented Jun 6, 2023 •

edited

Loading