From 668a6375f37b24108002bc2b03247e706eea3a82 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 16 Dec 2018 16:21:06 +0100 Subject: [PATCH 1/6] Fix #5578: Refine matches condition `matches` incorrectly returned a match if two members differed only in their type parameters. --- compiler/src/dotty/tools/dotc/core/Denotations.scala | 8 ++++++-- tests/neg/i5578.scala | 12 ++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 tests/neg/i5578.scala diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 15eb8c1e65a9..822caf92ddc1 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -1071,8 +1071,12 @@ object Denotations { final def matches(other: SingleDenotation)(implicit ctx: Context): Boolean = { val d = signature.matchDegree(other.signature) - d == Signature.FullMatch || - d >= Signature.ParamMatch && info.matches(other.info) + (// fast path: signatures are the same and neither denotation is a PolyType + // For polytypes, signatures alone do not tell us enough to be sure about matching. + d == Signature.FullMatch && !info.isInstanceOf[PolyType] && !other.info.isInstanceOf[PolyType] + || + // slow path + d >= Signature.ParamMatch && info.matches(other.info)) } def mapInherited(ownDenots: PreDenotation, prevDenots: PreDenotation, pre: Type)(implicit ctx: Context): SingleDenotation = diff --git a/tests/neg/i5578.scala b/tests/neg/i5578.scala new file mode 100644 index 000000000000..b6a1ec713aef --- /dev/null +++ b/tests/neg/i5578.scala @@ -0,0 +1,12 @@ +trait P[A]{ + def a[T]: A +} +class C extends P[Int]{ // error: class C needs to be abstract + def a = 1 +} +object O{ + def main(args: Array[String]) = { + val p: P[Int] = new C + println(p.a) + } +} \ No newline at end of file From 9553980a22bdce432d0106705545f5ead518987e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 16 Dec 2018 18:04:36 +0100 Subject: [PATCH 2/6] Further fixes - avoid forcing denotation's info in `matches` - deal with overloaded alternatives in assignType for TypeApply --- .../dotty/tools/dotc/core/Denotations.scala | 4 +- .../dotty/tools/dotc/typer/TypeAssigner.scala | 99 ++++++++++--------- tests/neg/i4819.scala | 8 +- tests/pos/i4819.scala | 8 +- 4 files changed, 63 insertions(+), 56 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 822caf92ddc1..4492e257bfc0 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -1073,9 +1073,9 @@ object Denotations { val d = signature.matchDegree(other.signature) (// fast path: signatures are the same and neither denotation is a PolyType // For polytypes, signatures alone do not tell us enough to be sure about matching. - d == Signature.FullMatch && !info.isInstanceOf[PolyType] && !other.info.isInstanceOf[PolyType] + d == Signature.FullMatch && + !infoOrCompleter.isInstanceOf[PolyType] && !other.infoOrCompleter.isInstanceOf[PolyType] || - // slow path d >= Signature.ParamMatch && info.matches(other.info)) } diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 17fc86a6c571..6471493e9132 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -381,65 +381,70 @@ trait TypeAssigner { } def assignType(tree: untpd.TypeApply, fn: Tree, args: List[Tree])(implicit ctx: Context): TypeApply = { - val ownType = fn.tpe.widen match { + def fail = tree.withType(errorType(err.takesNoParamsStr(fn, "type "), tree.pos)) + fn.tpe.widen match { case pt: TypeLambda => - val paramNames = pt.paramNames - if (hasNamedArg(args)) { - val paramBoundsByName = paramNames.zip(pt.paramInfos).toMap - - // Type arguments which are specified by name (immutable after this first loop) - val namedArgMap = new mutable.HashMap[Name, Type] - for (NamedArg(name, arg) <- args) - if (namedArgMap.contains(name)) - ctx.error(DuplicateNamedTypeParameter(name), arg.pos) - else if (!paramNames.contains(name)) - ctx.error(UndefinedNamedTypeParameter(name, paramNames), arg.pos) - else - namedArgMap(name) = arg.tpe - - // Holds indexes of non-named typed arguments in paramNames - val gapBuf = new mutable.ListBuffer[Int] - def nextPoly(idx: Int) = { - val newIndex = gapBuf.length - gapBuf += idx - // Re-index unassigned type arguments that remain after transformation - pt.paramRefs(newIndex) - } + tree.withType { + val paramNames = pt.paramNames + if (hasNamedArg(args)) { + val paramBoundsByName = paramNames.zip(pt.paramInfos).toMap + + // Type arguments which are specified by name (immutable after this first loop) + val namedArgMap = new mutable.HashMap[Name, Type] + for (NamedArg(name, arg) <- args) + if (namedArgMap.contains(name)) + ctx.error(DuplicateNamedTypeParameter(name), arg.pos) + else if (!paramNames.contains(name)) + ctx.error(UndefinedNamedTypeParameter(name, paramNames), arg.pos) + else + namedArgMap(name) = arg.tpe + + // Holds indexes of non-named typed arguments in paramNames + val gapBuf = new mutable.ListBuffer[Int] + def nextPoly(idx: Int) = { + val newIndex = gapBuf.length + gapBuf += idx + // Re-index unassigned type arguments that remain after transformation + pt.paramRefs(newIndex) + } - // Type parameters after naming assignment, conserving paramNames order - val normArgs: List[Type] = paramNames.zipWithIndex.map { case (pname, idx) => - namedArgMap.getOrElse(pname, nextPoly(idx)) - } + // Type parameters after naming assignment, conserving paramNames order + val normArgs: List[Type] = paramNames.zipWithIndex.map { case (pname, idx) => + namedArgMap.getOrElse(pname, nextPoly(idx)) + } - val transform = new TypeMap { - def apply(t: Type) = t match { - case TypeParamRef(`pt`, idx) => normArgs(idx) - case _ => mapOver(t) + val transform = new TypeMap { + def apply(t: Type) = t match { + case TypeParamRef(`pt`, idx) => normArgs(idx) + case _ => mapOver(t) + } + } + val resultType1 = transform(pt.resultType) + if (gapBuf.isEmpty) resultType1 + else { + val gaps = gapBuf.toList + pt.derivedLambdaType( + gaps.map(paramNames), + gaps.map(idx => transform(pt.paramInfos(idx)).bounds), + resultType1) } } - val resultType1 = transform(pt.resultType) - if (gapBuf.isEmpty) resultType1 else { - val gaps = gapBuf.toList - pt.derivedLambdaType( - gaps.map(paramNames), - gaps.map(idx => transform(pt.paramInfos(idx)).bounds), - resultType1) + val argTypes = args.tpes + if (sameLength(argTypes, paramNames)) pt.instantiate(argTypes) + else wrongNumberOfTypeArgs(fn.tpe, pt.typeParams, args, tree.pos) } } - else { - val argTypes = args.tpes - if (sameLength(argTypes, paramNames)) pt.instantiate(argTypes) - else wrongNumberOfTypeArgs(fn.tpe, pt.typeParams, args, tree.pos) - } case err: ErrorType => - err + tree.withType(err) + case ref: TermRef if ref.isOverloaded => + val disambiguated = ref.denot.suchThat(_.info.isInstanceOf[PolyType]) + if (disambiguated.exists) assignType(tree, fn.withType(ref.withDenot(disambiguated)), args) + else fail case _ => //println(i"bad type: $fn: ${fn.symbol} / ${fn.symbol.isType} / ${fn.symbol.info}") // DEBUG - errorType(err.takesNoParamsStr(fn, "type "), tree.pos) + fail } - - tree.withType(ownType) } def assignType(tree: untpd.Typed, tpt: Tree)(implicit ctx: Context): Typed = diff --git a/tests/neg/i4819.scala b/tests/neg/i4819.scala index 224682fc000c..8a9f89d45a13 100644 --- a/tests/neg/i4819.scala +++ b/tests/neg/i4819.scala @@ -7,10 +7,6 @@ trait Two[Y <: Foo] { } class Foo extends One[Foo] with Two[Foo] { - concat(0) // OK - - // TODO: This does not typecheck because the polymorphic overload is masked - // (we merge the denotations for both overloads into one and always prefer - // MethodType to PolyType, instead we should return a MultiDenotation). See #4819. - concat[Int](0) // error (that should actually not be an error) + concat(0) // error: ambiguous overload + concat[Int](0) // OK } diff --git a/tests/pos/i4819.scala b/tests/pos/i4819.scala index 508c579a8753..0494e9adfa55 100644 --- a/tests/pos/i4819.scala +++ b/tests/pos/i4819.scala @@ -7,6 +7,12 @@ trait Two[Y <: Foo] { } class Foo extends One[Foo] with Two[Foo] { - concat(0) // OK + concat[Int](0) // OK // See also tests/neg/i4819.scala } + +class Bar extends One[String] with Two[Foo] { + val x: String = concat(0) + val y = concat[Int](0) + val z: Foo = concat(0) +} \ No newline at end of file From 239ec1794b46b861c9e2c3b9bb1833c4c32f2282 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 16 Dec 2018 18:47:29 +0100 Subject: [PATCH 3/6] Drop invalid assertion We might now get more overloaded denotations than before, which only differ in type parameters. --- compiler/src/dotty/tools/dotc/core/Types.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 710edf35d78e..00ebbf7b4deb 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -1916,8 +1916,6 @@ object Types { setDenot(memberDenot(name, allowPrivate = !symbol.exists || symbol.is(Private))) private def setDenot(denot: Denotation)(implicit ctx: Context): Unit = { - if (ctx.isAfterTyper) - assert(!denot.isOverloaded || ctx.mode.is(Mode.Printing), this) if (Config.checkNoDoubleBindings) if (ctx.settings.YnoDoubleBindings.value) checkSymAssign(denot.symbol) From 6fa2a3ee2d7b385f33657f04ba410d2af3c677dd Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 16 Dec 2018 19:13:11 +0100 Subject: [PATCH 4/6] Refine type assignment of TypeApply Disambiguation with expected PolyType has to be done not only in the final type, but also in the function part of the tree. --- compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 6471493e9132..317d3f762696 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -439,7 +439,11 @@ trait TypeAssigner { tree.withType(err) case ref: TermRef if ref.isOverloaded => val disambiguated = ref.denot.suchThat(_.info.isInstanceOf[PolyType]) - if (disambiguated.exists) assignType(tree, fn.withType(ref.withDenot(disambiguated)), args) + if (disambiguated.exists) { + val fn1 = fn.withType(ref.withDenot(disambiguated)) + val tree1 = untpd.cpy.TypeApply(tree)(fn1, args) + assignType(tree1, fn1, args) + } else fail case _ => //println(i"bad type: $fn: ${fn.symbol} / ${fn.symbol.isType} / ${fn.symbol.info}") // DEBUG From 6c31e646d044c8ecdf7b574a1fbc852cfaa43381 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 17 Dec 2018 14:09:41 +0100 Subject: [PATCH 5/6] Test case for #5445 --- tests/neg/i5445.scala | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 tests/neg/i5445.scala diff --git a/tests/neg/i5445.scala b/tests/neg/i5445.scala new file mode 100644 index 000000000000..2cbe22d4ddd2 --- /dev/null +++ b/tests/neg/i5445.scala @@ -0,0 +1,6 @@ +object Test { + + trait A { def polymorphic[x]: Int } + val a = new A { val polymorphic = Unit } // error: object creation impossible + +} \ No newline at end of file From ab62df0de6a01968f2238d16dfc4c865d34cf63d Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 4 Jan 2019 22:34:43 +0100 Subject: [PATCH 6/6] Revert "Workaround #4819: Avoid creating incorrect JointRefDenotations" This partially reverts 4e695fb5559f28fcd95396a02bf8d778741a5c07 which is no longer needed after this PR (#5622), because we no longer try to merge in a single denotation a PolyType and a MethodType. --- compiler/src/dotty/tools/dotc/core/Denotations.scala | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 4492e257bfc0..e2568ad0461d 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -494,8 +494,6 @@ object Denotations { * boundary of sym1. For protected access, we count the enclosing * package as access boundary. * 5. sym1 is a method but sym2 is not. - * 6. sym1 is a non-polymorphic method but sym2 is a polymorphic method. - * (to be consistent with infoMeet, see #4819) * The aim of these criteria is to give some disambiguation on access which * - does not depend on textual order or other arbitrary choices * - minimizes raising of doubleDef errors @@ -510,7 +508,6 @@ object Denotations { accessBoundary(sym2).isProperlyContainedIn(accessBoundary(sym1)) || sym2.is(Bridge) && !sym1.is(Bridge) || sym1.is(Method) && !sym2.is(Method)) || - sym1.info.isInstanceOf[MethodType] && sym2.info.isInstanceOf[PolyType] || sym1.info.isErroneous) /** Sym preference provided types also override */