Make explicit arguments for context bounds an error from 3.5 #19316

odersky · 2023-12-20T19:10:20Z

Warning in 3.4. Rewrite available in 3.5-migration.

[test_scala2_library_tasty]

odersky · 2023-12-20T22:55:12Z

There's a difference in neg/hidden-type-errors and a failure in ReplTests. Both look like some internals of quotes pass
an explicit argument to a context bound parameter. It should be a using argument instead. @nicolasstucki , can you check?

odersky · 2023-12-23T17:44:02Z

@nicolasstucki There was an error in scala2-library-tasty-test. Can you advise what the reason could be?

- Error: /__w/dotty/dotty/scala2-library-tasty-tests/src/Main.scala:7:19 ------
Error:  7 |  case Red, Green, Blue
Error:    |                   ^
Error:    |Context bounds will map to context parameters.
Error:    |A `using` clause is needed to pass explicit arguments to them.
Error:    |This code can be rewritten automatically under -rewrite -source 3.5-migration.
Error:  one error found

nicolasstucki · 2024-01-03T08:27:19Z

@nicolasstucki There was an error in scala2-library-tasty-test. Can you advise what the reason could be?

- Error: /__w/dotty/dotty/scala2-library-tasty-tests/src/Main.scala:7:19 ------
Error:  7 |  case Red, Green, Blue
Error:    |                   ^
Error:    |Context bounds will map to context parameters.
Error:    |A `using` clause is needed to pass explicit arguments to them.
Error:    |This code can be rewritten automatically under -rewrite -source 3.5-migration.
Error:  one error found

The reason is that we compile the standard library with this new mode. In particular the def apply[T: ClassTag](xs: T*): Array[T] of Array now expects the using when the ClassTag argument is passed explicitly. This is happening in the desugaring of the enumeration in:

    private[this] val $values: Array[Color] =
      Array.apply[Color]([this.Red,this.Green,this.Blue : Color]*)(
        scala.reflect.ClassTag.apply[Color](classOf[Color]))

The solution is to make sure all desugarings that explicitly apply a context-bound set the ApplyKind to Using.

This particular error can be fixed in DesugarEnums.scala

private def ArrayLiteral(values: List[Tree], tpt: Tree)(using Context): Tree =
    val clazzOf = TypeApply(ref(defn.Predef_classOf.termRef), tpt :: Nil)
    val ctag    = Apply(TypeApply(ref(defn.ClassTagModule_apply.termRef), tpt :: Nil), clazzOf :: Nil)
    val apply   = Select(ref(defn.ArrayModule.termRef), nme.apply)
-   Apply(Apply(TypeApply(apply, tpt :: Nil), values), ctag :: Nil)
+   Apply(Apply(TypeApply(apply, tpt :: Nil), values), ctag :: Nil).setApplyKind(ApplyKind.Using)

I pushed this fix to this PR.

This fix makes scala2-library-tasty-tests/run and scala2-library-tasty-tests/test pass, though we might find other cases when running all tests with the TASTy library in test_scala2_library_tasty CI job. This one is usually tested on nightlies, I will enable it on this PR to make sure we are not missing something else.

odersky · 2024-01-07T09:58:23Z

@nicolasstucki Ready for review.

nicolasstucki · 2024-01-08T09:51:44Z

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

+      report.errorOrMigrationWarning(
+        em"""Context bounds will map to context parameters.
+            |A `using` clause is needed to pass explicit arguments to them.$rewriteMsg""",
+        tree.srcPos, MigrationVersion(`3.4`, `3.5`))


This migration version should be defined in MigrationVersion.scala as ExplicitContextBoundArgument.

nicolasstucki · 2024-01-08T09:53:23Z

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

+        em"""Context bounds will map to context parameters.
+            |A `using` clause is needed to pass explicit arguments to them.$rewriteMsg""",
+        tree.srcPos, MigrationVersion(`3.4`, `3.5`))
+      if sourceVersion.isAtLeast(`3.5-migration`) then


This should use ExplicitContextBoundArgument.needsPatch. It will use 3.4-migration and check that the version is -migrating.

nicolasstucki · 2024-01-08T09:56:12Z

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

+      && isContextBoundParams
+      && pt.applyKind != ApplyKind.Using
+    then
+      def rewriteMsg = Message.rewriteNotice("This code", `3.5-migration`)


This should use ExplicitContextBoundArgument. See comment bellow.

nicolasstucki · 2024-01-08T09:58:01Z

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

+    def remedy =
+      if ((prefix ++ suffix).isEmpty) "simply leave out the trailing ` _`"
+      else s"use `$prefix<function>$suffix` instead"
+    def rewrite = Message.rewriteNotice("This construct", `3.4-migration`)


This should be

Suggested change

def rewrite = Message.rewriteNotice("This construct", `3.4-migration`)

def rewrite = Message.rewriteNotice("This construct", MigrationVersion.FunctionUnderscore.warnVersion)

Maybe rewriteNotice should receive a MigrationVersion instead. I will check that later an refactor all the use cases if needed.

nicolasstucki · 2024-01-08T10:00:27Z

tests/neg/context-bounds-migration.scala

Tests with warnings should now be added to tests/warn.

tests/warn/context-bounds-migration.scala

class C[T] def foo[X: C] = () given [T]: C[T] = C[T]() def Test = foo(C[Int]()) // warn foo(using C[Int]()) // ok

nicolasstucki · 2024-01-08T10:10:55Z

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

+ *  and migrations.
+ */
+trait Migrations:
+  this: Typer =>


I am not convinced we should make this one a mixin. The one use case right now for typedAsFunction, all other functions that are here and could go in here seem to be static.

I would suggest making Migrations an object and redefining Typer.typedAsFunction as

def typedAsFunction(tree: untpd.PostfixOp, pt: Type)(using Context): Tree = migrate(Migrations.typedAsFunction(tree, pt), throw new AssertionError("can't retype a PostfixOp"))

I think there's nothing wrong with a mixin, and the hack of passing an explicit typer is worse. Right now it only applies tp typedAsFunction but I am sure that will not stay the only one.

nicolasstucki · 2024-01-08T10:17:07Z

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

@@ -158,6 +158,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
  // Overridden in derived typers
  def newLikeThis(nestingLevel: Int): Typer = new Typer(nestingLevel)

+  // Overridden to do nothing in derived typers


Suggested change

// Overridden to do nothing in derived typers

// Overridden to call `disabled` in derived typers

…5" (#19476) Backports #19316 to 3.4.0

Improves tests of scala#19316

Improves tests of #19316

odersky requested a review from nicolasstucki December 20, 2023 22:55

odersky assigned nicolasstucki Dec 20, 2023

odersky and others added 4 commits January 6, 2024 19:37

Make explicit arguments for context bounds an error from 3.5

02b4ec7

Don't run migration operations in ReTypers

41311bf

Fix test

697f107

Fix context bound application in enum desugaring

786852c

odersky force-pushed the context-bounds-migration branch from 2dd57a0 to 786852c Compare January 6, 2024 18:38

nicolasstucki added this to the 3.5.0 milestone Jan 8, 2024

nicolasstucki reviewed Jan 8, 2024

View reviewed changes

nicolasstucki assigned odersky and unassigned nicolasstucki Jan 8, 2024

odersky modified the milestones: 3.5.0, 3.4.0 Jan 8, 2024

Kordyjan added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Jan 8, 2024

Address review comments

13a71ef

odersky assigned nicolasstucki and unassigned odersky Jan 8, 2024

nicolasstucki approved these changes Jan 10, 2024

View reviewed changes

odersky merged commit fa93c73 into scala:main Jan 10, 2024
19 checks passed

odersky deleted the context-bounds-migration branch January 10, 2024 08:41

Kordyjan mentioned this pull request Jan 18, 2024

Backport "Make explicit arguments for context bounds an error from 3.5" #19476

Merged

Kordyjan added backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Jan 18, 2024

Kordyjan added a commit that referenced this pull request Jan 19, 2024

Backport "Make explicit arguments for context bounds an error from 3.…

5fe047b

…5" (#19476) Backports #19316 to 3.4.0

Kordyjan added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels Jan 19, 2024

nicolasstucki mentioned this pull request Jan 22, 2024

Use 3.4.0-RC2 for MiMa checks #19503

Closed

nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 22, 2024

Add tests for context bounds migration

5d5ce1a

Improves tests of scala#19316

nicolasstucki mentioned this pull request Jan 22, 2024

Add tests for context bounds migration #19510

Merged

nicolasstucki added a commit that referenced this pull request Jan 23, 2024

Add tests for context bounds migration (#19510)

c78c6d0

Improves tests of #19316

Kordyjan pushed a commit that referenced this pull request Jan 23, 2024

Add tests for context bounds migration

c76806c

Improves tests of #19316

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make explicit arguments for context bounds an error from 3.5 #19316

Make explicit arguments for context bounds an error from 3.5 #19316

odersky commented Dec 20, 2023 •

edited by nicolasstucki

Loading

odersky commented Dec 20, 2023

odersky commented Dec 23, 2023

nicolasstucki commented Jan 3, 2024 •

edited

Loading

odersky commented Jan 7, 2024

nicolasstucki Jan 8, 2024

nicolasstucki Jan 8, 2024

nicolasstucki Jan 8, 2024

nicolasstucki Jan 8, 2024

nicolasstucki Jan 8, 2024

nicolasstucki Jan 8, 2024

nicolasstucki Jan 8, 2024

odersky Jan 8, 2024

nicolasstucki Jan 8, 2024

	def rewrite = Message.rewriteNotice("This construct", `3.4-migration`)
	def rewrite = Message.rewriteNotice("This construct", MigrationVersion.FunctionUnderscore.warnVersion)

	// Overridden to do nothing in derived typers
	// Overridden to call `disabled` in derived typers

Make explicit arguments for context bounds an error from 3.5 #19316

Make explicit arguments for context bounds an error from 3.5 #19316

Conversation

odersky commented Dec 20, 2023 • edited by nicolasstucki Loading

odersky commented Dec 20, 2023

odersky commented Dec 23, 2023

nicolasstucki commented Jan 3, 2024 • edited Loading

odersky commented Jan 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odersky commented Dec 20, 2023 •

edited by nicolasstucki

Loading

nicolasstucki commented Jan 3, 2024 •

edited

Loading