Skip to content

Commit

Permalink
Fix Scala Wart about implicit () class parameters
Browse files Browse the repository at this point in the history
Fixes scala#2576

As the discussion in scala#2576 shows, we still have some problems with the implicitly
inserted empty parameter lists for class constructors. We do need that empty list
to support syntax like `C()` and `new C()`. But it gets in the way if a class has
using clauses. Example from the issue:
```scala
class Bar(using x: Int)(y: String)
given Int = ???
def test = new Bar("")
```
Here, an implicitly inserted `()` in front makes the last line fail. We'd need
`new Bar()("")`.

If a class has only using clauses as parameters we now insert a `()` at the end
instead of at the start. That makes the example compile.

For old-style implicit parameters we don't have a choice. We still need the `()` at
the start since otherwise we'd change the meaning of calls with explicit arguments
for the implicit parameters.
  • Loading branch information
odersky authored and michelou committed Apr 25, 2022
1 parent 18861cb commit 74c9e93
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 40 deletions.
14 changes: 9 additions & 5 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -589,12 +589,16 @@ object desugar {

// new C[Ts](paramss)
lazy val creatorExpr = {
val vparamss = constrVparamss match {
case (vparam :: _) :: _ if vparam.mods.isOneOf(GivenOrImplicit) => // add a leading () to match class parameters
val vparamss = constrVparamss match
case (vparam :: _) :: _ if vparam.mods.is(Implicit) => // add a leading () to match class parameters
Nil :: constrVparamss
case _ =>
constrVparamss
}
if constrVparamss.nonEmpty && constrVparamss.forall {
case vparam :: _ => vparam.mods.is(Given)
case _ => false
}
then constrVparamss :+ Nil // add a trailing () to match class parameters
else constrVparamss
val nu = vparamss.foldLeft(makeNew(classTypeRef)) { (nu, vparams) =>
val app = Apply(nu, vparams.map(refOfDef))
vparams match {
Expand Down Expand Up @@ -822,7 +826,7 @@ object desugar {
}

flatTree(cdef1 :: companions ::: implicitWrappers ::: enumScaffolding)
}.showing(i"desugared: $result", Printers.desugar)
}.showing(i"desugared: $cdef --> $result", Printers.desugar)

/** Expand
*
Expand Down
8 changes: 6 additions & 2 deletions compiler/src/dotty/tools/dotc/config/PathResolver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ object PathResolver {

def fromPathString(path: String)(using Context): ClassPath = {
val settings = ctx.settings.classpath.update(path)
new PathResolver()(using ctx.fresh.setSettings(settings)).result
inContext(ctx.fresh.setSettings(settings)) {
new PathResolver().result
}
}

/** Show values in Environment and Defaults when no argument is provided.
Expand All @@ -147,7 +149,9 @@ object PathResolver {
val ArgsSummary(sstate, rest, errors, warnings) =
ctx.settings.processArguments(args.toList, true, ctx.settingsState)
errors.foreach(println)
val pr = new PathResolver()(using ctx.fresh.setSettings(sstate))
val pr = inContext(ctx.fresh.setSettings(sstate)) {
new PathResolver()
}
println(" COMMAND: 'scala %s'".format(args.mkString(" ")))
println("RESIDUAL: 'scala %s'\n".format(rest.mkString(" ")))

Expand Down
17 changes: 13 additions & 4 deletions compiler/src/dotty/tools/dotc/core/NamerOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,23 @@ object NamerOps:
case TypeSymbols(tparams) :: _ => ctor.owner.typeRef.appliedTo(tparams.map(_.typeRef))
case _ => ctor.owner.typeRef

/** if isConstructor, make sure it has one leading non-implicit parameter list */
/** If isConstructor, make sure it has at least one non-implicit parameter list
* This is done by adding a () in front of a leading old style implicit parameter,
* or by adding a () as last -- or only -- parameter list if the constructor has
* only using clauses as parameters.
*/
def normalizeIfConstructor(paramss: List[List[Symbol]], isConstructor: Boolean)(using Context): List[List[Symbol]] =
if !isConstructor then paramss
else paramss match
case Nil :: _ => paramss
case TermSymbols(vparam :: _) :: _ if !vparam.isOneOf(GivenOrImplicit) => paramss
case TypeSymbols(tparams) :: paramss1 => tparams :: normalizeIfConstructor(paramss1, isConstructor)
case _ => Nil :: paramss
case TermSymbols(vparam :: _) :: _ if vparam.is(Implicit) => Nil :: paramss
case _ =>
if paramss.forall {
case TermSymbols(vparams) => vparams.nonEmpty && vparams.head.is(Given)
case _ => true
}
then paramss :+ Nil
else paramss

/** The method type corresponding to given parameters and result type */
def methodType(paramss: List[List[Symbol]], resultType: Type, isJava: Boolean = false)(using Context): Type =
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Bridges.scala
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
* time deferred methods in `stats` that are replaced by a bridge with the same signature.
*/
def add(stats: List[untpd.Tree]): List[untpd.Tree] =
val opc = new BridgesCursor()(using preErasureCtx)
val opc = inContext(preErasureCtx) { new BridgesCursor }
while opc.hasNext do
if !opc.overriding.is(Deferred) then
addBridgeIfNeeded(opc.overriding, opc.overridden)
Expand Down
37 changes: 22 additions & 15 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2381,25 +2381,32 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer

/** If `ref` is an implicitly parameterized trait, pass an implicit argument list.
* Otherwise, if `ref` is a parameterized trait, error.
* Note: Traits and classes currently always have at least an empty parameter list ()
* before the implicit parameters (this is inserted if not given in source).
* We skip this parameter list when deciding whether a trait is parameterless or not.
* Note: Traits and classes have sometimes a synthesized empty parameter list ()
* in front or after the implicit parameter(s). See NamerOps.normalizeIfConstructor.
* We synthesize a () argument at the correct place in this case.
* @param ref The tree referring to the (parent) trait
* @param psym Its type symbol
* @param cinfo The info of its constructor
*/
def maybeCall(ref: Tree, psym: Symbol): Tree = psym.primaryConstructor.info.stripPoly match
case cinfo @ MethodType(Nil) if cinfo.resultType.isImplicitMethod =>
def maybeCall(ref: Tree, psym: Symbol): Tree =
def appliedRef =
typedExpr(untpd.New(untpd.TypedSplice(ref)(using superCtx), Nil))(using superCtx)
case cinfo @ MethodType(Nil) if !cinfo.resultType.isInstanceOf[MethodType] =>
ref
case cinfo: MethodType =>
if !ctx.erasedTypes then // after constructors arguments are passed in super call.
typr.println(i"constr type: $cinfo")
report.error(ParameterizedTypeLacksArguments(psym), ref.srcPos)
ref
case _ =>
ref
def dropContextual(tp: Type): Type = tp.stripPoly match
case mt: MethodType if mt.isContextualMethod => dropContextual(mt.resType)
case _ => tp
psym.primaryConstructor.info.stripPoly match
case cinfo @ MethodType(Nil)
if cinfo.resultType.isImplicitMethod && !cinfo.resultType.isContextualMethod =>
appliedRef
case cinfo =>
val cinfo1 = dropContextual(cinfo)
cinfo1 match
case cinfo1 @ MethodType(Nil) if !cinfo1.resultType.isInstanceOf[MethodType] =>
if cinfo1 ne cinfo then appliedRef else ref
case cinfo1: MethodType if !ctx.erasedTypes =>
report.error(ParameterizedTypeLacksArguments(psym), ref.srcPos)
ref
case _ =>
ref

val seenParents = mutable.Set[Symbol]()

Expand Down
20 changes: 10 additions & 10 deletions tests/neg/i12344.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import scala.quoted.*

class C(using q: Quotes)(i: Int = 1, f: q.reflect.Flags = q.reflect.Flags.EmptyFlags)

def test1a(using q: Quotes) = new C() // error
def test2a(using q: Quotes) = new C(1) // error
def test3a(using q: Quotes) = new C(1, q.reflect.Flags.Lazy) // error
def test4a(using q: Quotes) = new C(f = q.reflect.Flags.Lazy) // error
def test1a(using q: Quotes) = new C()
def test2a(using q: Quotes) = new C(1)
def test3a(using q: Quotes) = new C(1, q.reflect.Flags.Lazy)
def test4a(using q: Quotes) = new C(f = q.reflect.Flags.Lazy)

def test1b(using q: Quotes) = C() // error
def test2b(using q: Quotes) = C(1) // error
def test3b(using q: Quotes) = C(1, q.reflect.Flags.Lazy) // error
def test4b(using q: Quotes) = C(f = q.reflect.Flags.Lazy) // error
def test1b(using q: Quotes) = C()
def test2b(using q: Quotes) = C(1)
def test3b(using q: Quotes) = C(1, q.reflect.Flags.Lazy)
def test4b(using q: Quotes) = C(f = q.reflect.Flags.Lazy)

def test1c(using q: Quotes) = new C(using q)()
def test2c(using q: Quotes) = new C(using q)(1)
Expand All @@ -22,5 +22,5 @@ def test2d(using q: Quotes) = C(using q)(1)
def test3d(using q: Quotes) = C(using q)(1, q.reflect.Flags.Lazy)
def test4d(using q: Quotes) = C(using q)(f = q.reflect.Flags.Lazy)

def test1e(using q: Quotes) = new C()()
def test2e(using q: Quotes) = C()()
def test1e(using q: Quotes) = new C()() // error
def test2e(using q: Quotes) = C()() // error
1 change: 0 additions & 1 deletion tests/pos/given-constrapps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ class Foo(using TC) {

object Test extends App {
new C(using tc)
new C()(using tc)
new C(using tc) {}
new C2(1)(using tc)(using List(tc))
new C2(1)(using tc)(using List(tc)) {}
Expand Down
3 changes: 3 additions & 0 deletions tests/pos/i2576.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Bar(using x: Int)(y: String)
given Int = ???
def test = new Bar("")
1 change: 1 addition & 0 deletions tests/run-macros/i12021/Macro_1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ def inspect2[A: Type](using Quotes): Expr[String] = {
val ps =
TypeRepr.of[A].typeSymbol.primaryConstructor.tree match
case DefDef(_, List(Nil, ps: TermParamClause), _, _) => ps
case DefDef(_, List(ps: TermParamClause, Nil), _, _) => ps
case DefDef(_, List(ps: TermParamClause), _, _) => ps

val names = ps.params.map(p => s"${p.name}: ${p.tpt.show}").mkString("(", ", ", ")")
Expand Down
4 changes: 2 additions & 2 deletions tests/run/i2567.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ object Test extends App {
new Foo
new Foo(using tc)
new Foo()
new Foo()(using tc)
new Foo(using tc)
Foo()
Foo()(using tc)
Foo(using tc)
}

0 comments on commit 74c9e93

Please sign in to comment.