-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Make explicit arguments for context bounds an error from 3.5 #19316
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
Changes from 4 commits
02b4ec7
41311bf
697f107
786852c
13a71ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,108 @@ | ||||||
package dotty.tools | ||||||
package dotc | ||||||
package typer | ||||||
|
||||||
import core.* | ||||||
import ast.* | ||||||
import Contexts.* | ||||||
import Types.* | ||||||
import Flags.* | ||||||
import Names.* | ||||||
import StdNames.* | ||||||
import Symbols.* | ||||||
import Trees.* | ||||||
import ProtoTypes.* | ||||||
import Decorators.* | ||||||
import config.MigrationVersion | ||||||
import config.Feature.{sourceVersion, migrateTo3} | ||||||
import config.SourceVersion.* | ||||||
import reporting.* | ||||||
import NameKinds.ContextBoundParamName | ||||||
import rewrites.Rewrites.patch | ||||||
import util.Spans.Span | ||||||
import rewrites.Rewrites | ||||||
|
||||||
/** A utility trait containing source-dependent deprecation messages | ||||||
* and migrations. | ||||||
*/ | ||||||
trait Migrations: | ||||||
this: Typer => | ||||||
|
||||||
import tpd.* | ||||||
|
||||||
/** Flag & migrate `?` used as a higher-kinded type parameter | ||||||
* Warning in 3.0-migration, error from 3.0 | ||||||
*/ | ||||||
def kindProjectorQMark(tree: untpd.TypeDef, sym: Symbol)(using Context): Unit = | ||||||
if tree.name eq tpnme.? then | ||||||
val addendum = if sym.owner.is(TypeParam) | ||||||
then ", use `_` to denote a higher-kinded type parameter" | ||||||
else "" | ||||||
val namePos = tree.sourcePos.withSpan(tree.nameSpan) | ||||||
report.errorOrMigrationWarning( | ||||||
em"`?` is not a valid type name$addendum", namePos, MigrationVersion.Scala2to3) | ||||||
|
||||||
def typedAsFunction(tree: untpd.PostfixOp, pt: Type)(using Context): Tree = { | ||||||
val untpd.PostfixOp(qual, Ident(nme.WILDCARD)) = tree: @unchecked | ||||||
val pt1 = if (defn.isFunctionNType(pt)) pt else AnyFunctionProto | ||||||
val nestedCtx = ctx.fresh.setNewTyperState() | ||||||
val res = typed(qual, pt1)(using nestedCtx) | ||||||
res match { | ||||||
case closure(_, _, _) => | ||||||
case _ => | ||||||
val recovered = typed(qual)(using ctx.fresh.setExploreTyperState()) | ||||||
val msg = OnlyFunctionsCanBeFollowedByUnderscore(recovered.tpe.widen, tree) | ||||||
report.errorOrMigrationWarning(msg, tree.srcPos, MigrationVersion.Scala2to3) | ||||||
if MigrationVersion.Scala2to3.needsPatch then | ||||||
// Under -rewrite, patch `x _` to `(() => x)` | ||||||
msg.actions | ||||||
.headOption | ||||||
.foreach(Rewrites.applyAction) | ||||||
return typed(untpd.Function(Nil, qual), pt) | ||||||
} | ||||||
nestedCtx.typerState.commit() | ||||||
|
||||||
lazy val (prefix, suffix) = res match { | ||||||
case Block(mdef @ DefDef(_, vparams :: Nil, _, _) :: Nil, _: Closure) => | ||||||
val arity = vparams.length | ||||||
if (arity > 0) ("", "") else ("(() => ", "())") | ||||||
case _ => | ||||||
("(() => ", ")") | ||||||
} | ||||||
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`) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe |
||||||
report.errorOrMigrationWarning( | ||||||
em"""The syntax `<function> _` is no longer supported; | ||||||
|you can $remedy$rewrite""", | ||||||
tree.srcPos, | ||||||
MigrationVersion.FunctionUnderscore) | ||||||
if MigrationVersion.FunctionUnderscore.needsPatch then | ||||||
patch(Span(tree.span.start), prefix) | ||||||
patch(Span(qual.span.end, tree.span.end), suffix) | ||||||
|
||||||
res | ||||||
} | ||||||
|
||||||
/** Flag & migrate explicit normal arguments to parameters coming from context bounds | ||||||
* Warning in 3.4, error in 3.5, rewrite in 3.5-migration. | ||||||
*/ | ||||||
def contextBoundParams(tree: Tree, tp: Type, pt: FunProto)(using Context): Unit = | ||||||
def isContextBoundParams = tp.stripPoly match | ||||||
case MethodType(ContextBoundParamName(_) :: _) => true | ||||||
case _ => false | ||||||
if sourceVersion.isAtLeast(`3.4`) | ||||||
&& isContextBoundParams | ||||||
&& pt.applyKind != ApplyKind.Using | ||||||
then | ||||||
def rewriteMsg = Message.rewriteNotice("This code", `3.5-migration`) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should use |
||||||
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`)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This migration version should be defined in |
||||||
if sourceVersion.isAtLeast(`3.5-migration`) then | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should use |
||||||
patch(Span(pt.args.head.span.start), "using ") | ||||||
end contextBoundParams | ||||||
|
||||||
end Migrations |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -43,7 +43,7 @@ import config.Printers.{gadts, typr} | |||||
import config.Feature | ||||||
import config.Feature.{sourceVersion, migrateTo3} | ||||||
import config.SourceVersion.* | ||||||
import rewrites.Rewrites.patch | ||||||
import rewrites.Rewrites, Rewrites.patch | ||||||
import staging.StagingLevel | ||||||
import reporting.* | ||||||
import Nullables.* | ||||||
|
@@ -53,7 +53,6 @@ import config.Config | |||||
import config.MigrationVersion | ||||||
|
||||||
import scala.annotation.constructorOnly | ||||||
import dotty.tools.dotc.rewrites.Rewrites | ||||||
|
||||||
object Typer { | ||||||
|
||||||
|
@@ -127,7 +126,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |||||
with Dynamic | ||||||
with Checking | ||||||
with QuotesAndSplices | ||||||
with Deriving { | ||||||
with Deriving | ||||||
with Migrations { | ||||||
|
||||||
import Typer.* | ||||||
import tpd.{cpy => _, _} | ||||||
|
@@ -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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
protected def migrate[T](migration: => T, disabled: => T = ()): T = migration | ||||||
|
||||||
/** Find the type of an identifier with given `name` in given context `ctx`. | ||||||
* @param name the name of the identifier | ||||||
* @param pt the expected type | ||||||
|
@@ -2978,48 +2981,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |||||
else tree1 | ||||||
} | ||||||
|
||||||
def typedAsFunction(tree: untpd.PostfixOp, pt: Type)(using Context): Tree = { | ||||||
val untpd.PostfixOp(qual, Ident(nme.WILDCARD)) = tree: @unchecked | ||||||
val pt1 = if (defn.isFunctionNType(pt)) pt else AnyFunctionProto | ||||||
val nestedCtx = ctx.fresh.setNewTyperState() | ||||||
val res = typed(qual, pt1)(using nestedCtx) | ||||||
res match { | ||||||
case closure(_, _, _) => | ||||||
case _ => | ||||||
val recovered = typed(qual)(using ctx.fresh.setExploreTyperState()) | ||||||
val msg = OnlyFunctionsCanBeFollowedByUnderscore(recovered.tpe.widen, tree) | ||||||
report.errorOrMigrationWarning(msg, tree.srcPos, MigrationVersion.Scala2to3) | ||||||
if MigrationVersion.Scala2to3.needsPatch then | ||||||
// Under -rewrite, patch `x _` to `(() => x)` | ||||||
msg.actions | ||||||
.headOption | ||||||
.foreach(Rewrites.applyAction) | ||||||
return typed(untpd.Function(Nil, qual), pt) | ||||||
} | ||||||
nestedCtx.typerState.commit() | ||||||
|
||||||
lazy val (prefix, suffix) = res match { | ||||||
case Block(mdef @ DefDef(_, vparams :: Nil, _, _) :: Nil, _: Closure) => | ||||||
val arity = vparams.length | ||||||
if (arity > 0) ("", "") else ("(() => ", "())") | ||||||
case _ => | ||||||
("(() => ", ")") | ||||||
} | ||||||
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`) | ||||||
report.errorOrMigrationWarning( | ||||||
em"""The syntax `<function> _` is no longer supported; | ||||||
|you can $remedy$rewrite""", | ||||||
tree.srcPos, | ||||||
MigrationVersion.FunctionUnderscore) | ||||||
if MigrationVersion.FunctionUnderscore.needsPatch then | ||||||
patch(Span(tree.span.start), prefix) | ||||||
patch(Span(qual.span.end, tree.span.end), suffix) | ||||||
|
||||||
res | ||||||
} | ||||||
override def typedAsFunction(tree: untpd.PostfixOp, pt: Type)(using Context): Tree = | ||||||
migrate(super.typedAsFunction(tree, pt), throw new AssertionError("can't retype a PostfixOp")) | ||||||
|
||||||
/** Translate infix operation expression `l op r` to | ||||||
* | ||||||
|
@@ -3137,13 +3100,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |||||
case tree: untpd.TypeDef => | ||||||
// separate method to keep dispatching method `typedNamed` short which might help the JIT | ||||||
def typedTypeOrClassDef: Tree = | ||||||
if tree.name eq tpnme.? then | ||||||
val addendum = if sym.owner.is(TypeParam) | ||||||
then ", use `_` to denote a higher-kinded type parameter" | ||||||
else "" | ||||||
val namePos = tree.sourcePos.withSpan(tree.nameSpan) | ||||||
report.errorOrMigrationWarning( | ||||||
em"`?` is not a valid type name$addendum", namePos, MigrationVersion.Scala2to3) | ||||||
migrate(kindProjectorQMark(tree, sym)) | ||||||
if tree.isClassDef then | ||||||
typedClassDef(tree, sym.asClass)(using ctx.localContext(tree, sym)) | ||||||
else | ||||||
|
@@ -3818,24 +3775,12 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |||||
def adaptToArgs(wtp: Type, pt: FunProto): Tree = wtp match { | ||||||
case wtp: MethodOrPoly => | ||||||
def methodStr = methPart(tree).symbol.showLocated | ||||||
if (matchingApply(wtp, pt)) | ||||||
if matchingApply(wtp, pt) then | ||||||
migrate(contextBoundParams(tree, wtp, pt)) | ||||||
if needsTupledDual(wtp, pt) then adapt(tree, pt.tupledDual, locked) | ||||||
else tree | ||||||
else if wtp.isContextualMethod then | ||||||
def isContextBoundParams = wtp.stripPoly match | ||||||
case MethodType(ContextBoundParamName(_) :: _) => true | ||||||
case _ => false | ||||||
if sourceVersion == `future-migration` && isContextBoundParams && pt.args.nonEmpty | ||||||
then // Under future-migration, don't infer implicit arguments yet for parameters | ||||||
// coming from context bounds. Issue a warning instead and offer a patch. | ||||||
def rewriteMsg = Message.rewriteNotice("This code", `future-migration`) | ||||||
report.migrationWarning( | ||||||
em"""Context bounds will map to context parameters. | ||||||
|A `using` clause is needed to pass explicit arguments to them.$rewriteMsg""", tree.srcPos) | ||||||
patch(Span(pt.args.head.span.start), "using ") | ||||||
tree | ||||||
else | ||||||
adaptNoArgs(wtp) // insert arguments implicitly | ||||||
adaptNoArgs(wtp) // insert arguments implicitly | ||||||
else if (tree.symbol.isPrimaryConstructor && tree.symbol.info.firstParamTypes.isEmpty) | ||||||
readapt(tree.appliedToNone) // insert () to primary constructors | ||||||
else | ||||||
|
@@ -4441,7 +4386,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |||||
protected def matchingApply(methType: MethodOrPoly, pt: FunProto)(using Context): Boolean = | ||||||
val isUsingApply = pt.applyKind == ApplyKind.Using | ||||||
methType.isContextualMethod == isUsingApply | ||||||
|| methType.isImplicitMethod && isUsingApply // for a transition allow `with` arguments for regular implicit parameters | ||||||
|| methType.isImplicitMethod && isUsingApply // for a transition allow `using` arguments for regular implicit parameters | ||||||
|
||||||
/** Check that `tree == x: pt` is typeable. Used when checking a pattern | ||||||
* against a selector of type `pt`. This implementation accounts for | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests with warnings should now be added to
class C[T]
def foo[X: C] = ()
given [T]: C[T] = C[T]()
def Test =
foo(C[Int]()) // warn
foo(using C[Int]()) // ok |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
//> using options -Xfatal-warnings | ||
|
||
class C[T] | ||
def foo[X: C] = () | ||
|
||
given [T]: C[T] = C[T]() | ||
|
||
def Test = | ||
foo(C[Int]()) // error | ||
foo(using C[Int]()) // ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
anobject
and redefiningTyper.typedAsFunction
asThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.