From 3fad1e666b11ff34a83c69340e42fdd80b2bcc2f Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 3 Aug 2021 14:37:22 -0700 Subject: [PATCH 1/2] Repeated params must correspond in override Refchecks runs after elimRepeated and did not error on an attempt to override RepeatedParam with Seq. Also show RepeatedParam in error message. --- .../dotty/tools/dotc/core/Decorators.scala | 16 ++++++++++++++ .../tools/dotc/typer/ErrorReporting.scala | 3 ++- .../dotty/tools/dotc/typer/RefChecks.scala | 20 +++++++++++------- tests/neg/override-scala2-macro.check | 4 ++-- tests/neg/overrides.scala | 21 +++++++++++++++++++ 5 files changed, 54 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Decorators.scala b/compiler/src/dotty/tools/dotc/core/Decorators.scala index 4ef0dbc9a43b..1bb9ba9f6227 100644 --- a/compiler/src/dotty/tools/dotc/core/Decorators.scala +++ b/compiler/src/dotty/tools/dotc/core/Decorators.scala @@ -234,6 +234,22 @@ object Decorators { def nestedExists(p: T => Boolean): Boolean = xss match case xs :: xss1 => xs.exists(p) || xss1.nestedExists(p) case nil => false + def nestedZipExists(yss: List[List[U]])(f: (T, U) => Boolean): Boolean = + @tailrec def zipExists(p1: List[T], p2: List[U]): Boolean = + p1 match + case h :: t => + p2 match + case h2 :: t2 => f(h, h2) || zipExists(t, t2) + case _ => false + case _ => false + @tailrec def loop(ps1: List[List[T]], ps2: List[List[U]]): Boolean = + ps1 match + case h :: t => + ps2 match + case h2 :: t2 => zipExists(h, h2) || loop(t, t2) + case _ => false + case _ => false + loop(xss, yss) end extension extension (text: Text) diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index 126d109889e1..30d86e3a1f8c 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -137,7 +137,7 @@ object ErrorReporting { /** Explain info of symbol `sym` as a member of class `base`. * @param showLocation if true also show sym's location. */ - def infoString(sym: Symbol, base: Type, showLocation: Boolean): String = + def infoString(sym: Symbol, base: Type, showLocation: Boolean): String = atPhase(Phases.typerPhase) { val sym1 = sym.underlyingSymbol def info = base.memberInfo(sym1) val infoStr = @@ -147,6 +147,7 @@ object ErrorReporting { else if sym1.isTerm then i" of type $info" else "" i"${if showLocation then sym1.showLocated else sym1}$infoStr" + } def infoStringWithLocation(sym: Symbol, base: Type) = infoString(sym, base, showLocation = true) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 73331046b41f..52b210dd17ea 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -336,6 +336,11 @@ object RefChecks { next == other || isInheritedAccessor(next, other) } + def incompatibleRepeatedParam(member: Symbol, other: Symbol): Boolean = + member.is(Method, butNot = JavaDefined) && other.is(Method, butNot = JavaDefined) && atPhase(typerPhase) { + member.info.paramInfoss.nestedZipExists(other.info.paramInfoss)(_.isRepeatedParam != _.isRepeatedParam) + } + /* Check that all conditions for overriding `other` by `member` * of class `clazz` are met. */ @@ -425,10 +430,8 @@ object RefChecks { //Console.println(infoString(member) + " overrides " + infoString(other) + " in " + clazz);//DEBUG /* Is the intersection between given two lists of overridden symbols empty? */ - def intersectionIsEmpty(syms1: Iterator[Symbol], syms2: Iterator[Symbol]) = { - val set2 = syms2.toSet - !(syms1 exists (set2 contains _)) - } + def intersectionIsEmpty(syms1: Iterator[Symbol], syms2: Iterator[Symbol]) = + !syms1.exists(syms2.toSet.contains) // o: public | protected | package-protected (aka java's default access) // ^-may be overridden by member with access privileges-v @@ -498,6 +501,8 @@ object RefChecks { + "\n(Note: this can be resolved by declaring an override in " + clazz + ".)") else if member.is(Exported) then overrideError("cannot override since it comes from an export") + else if incompatibleRepeatedParam(member, other) then + overrideError("cannot override because erased signatures conflict in repeated parameter") else overrideError("needs `override` modifier") else if (other.is(AbsOverride) && other.isIncompleteIn(clazz) && !member.is(AbsOverride)) @@ -878,6 +883,7 @@ object RefChecks { def isSignatureMatch(sym: Symbol) = sym.isType || { val self = clazz.thisType sym.asSeenFrom(self).matches(member.asSeenFrom(self)) + && !incompatibleRepeatedParam(sym, member) } /* The rules for accessing members which have an access boundary are more @@ -910,8 +916,8 @@ object RefChecks { } // 4. Check that every defined member with an `override` modifier overrides some other member. - for (member <- clazz.info.decls) - if (member.isAnyOverride && !(clazz.thisType.baseClasses exists (hasMatchingSym(_, member)))) { + for member <- clazz.info.decls do + if member.isAnyOverride && !clazz.thisType.baseClasses.exists(hasMatchingSym(_, member)) then if (checks != noPrinter) for (bc <- clazz.info.baseClasses.tail) { val sym = bc.info.decl(member.name).symbol @@ -935,7 +941,7 @@ object RefChecks { } member.resetFlag(Override) member.resetFlag(AbsOverride) - } + end if } /** Check that we do not "override" anything with a private method diff --git a/tests/neg/override-scala2-macro.check b/tests/neg/override-scala2-macro.check index ff5e478342dd..fadf3a003402 100644 --- a/tests/neg/override-scala2-macro.check +++ b/tests/neg/override-scala2-macro.check @@ -1,5 +1,5 @@ -- [E164] Declaration Error: tests/neg/override-scala2-macro.scala:2:22 ------------------------------------------------ 2 | override inline def f[A >: Any](args: A*): String = ??? // error | ^ - |error overriding method f in class StringContext of type [A >: Any](args: Seq[A]): String; - | method f of type [A >: Any](args: Seq[A]): String cannot be used here - only Scala-2 macros can override Scala-2 macros + |error overriding method f in class StringContext of type [A >: Any](args: A*): String; + | method f of type [A >: Any](args: A*): String cannot be used here - only Scala-2 macros can override Scala-2 macros diff --git a/tests/neg/overrides.scala b/tests/neg/overrides.scala index c8fc8de97f7c..ff83b91d26be 100644 --- a/tests/neg/overrides.scala +++ b/tests/neg/overrides.scala @@ -120,3 +120,24 @@ class C extends A { override def m: Int = 42 // error: has incompatible type } } + +package p6 { + class A { def apply(xs: Int*) = 42 } + class B extends A { override def apply(xs: Seq[Int]) = 42 } // error +} +package p7 { + class A { def apply(xs: Int*) = 42 } + class B extends A { def apply(xs: Seq[Int]) = 42 } // error +} +package p8 { + class A { def apply(xs: Seq[Int]) = 42 } + class B extends A { override def apply(xs: Int*) = 42 } // error +} +package p9 { + class A { def apply(xs: Seq[Int]) = 42 } + class B extends A { def apply(xs: Int*) = 42 } // error +} +package p10 { + class A { def apply(s: String)(xs: Int*) = 42 } + class B extends A { def apply(s: String)(xs: Seq[Int]) = 42 } // error +} From d6fd2c4de88c0d142797bf29eccad7b565c037de Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 13 Feb 2023 00:40:08 -0800 Subject: [PATCH 2/2] Only check last position, use DoubleDefinition --- .../dotty/tools/dotc/core/Decorators.scala | 16 ---------------- .../tools/dotc/typer/ErrorReporting.scala | 3 +-- .../dotty/tools/dotc/typer/RefChecks.scala | 19 +++++++++++++++---- tests/neg/override-scala2-macro.check | 4 ++-- 4 files changed, 18 insertions(+), 24 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Decorators.scala b/compiler/src/dotty/tools/dotc/core/Decorators.scala index 1bb9ba9f6227..4ef0dbc9a43b 100644 --- a/compiler/src/dotty/tools/dotc/core/Decorators.scala +++ b/compiler/src/dotty/tools/dotc/core/Decorators.scala @@ -234,22 +234,6 @@ object Decorators { def nestedExists(p: T => Boolean): Boolean = xss match case xs :: xss1 => xs.exists(p) || xss1.nestedExists(p) case nil => false - def nestedZipExists(yss: List[List[U]])(f: (T, U) => Boolean): Boolean = - @tailrec def zipExists(p1: List[T], p2: List[U]): Boolean = - p1 match - case h :: t => - p2 match - case h2 :: t2 => f(h, h2) || zipExists(t, t2) - case _ => false - case _ => false - @tailrec def loop(ps1: List[List[T]], ps2: List[List[U]]): Boolean = - ps1 match - case h :: t => - ps2 match - case h2 :: t2 => zipExists(h, h2) || loop(t, t2) - case _ => false - case _ => false - loop(xss, yss) end extension extension (text: Text) diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index 30d86e3a1f8c..126d109889e1 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -137,7 +137,7 @@ object ErrorReporting { /** Explain info of symbol `sym` as a member of class `base`. * @param showLocation if true also show sym's location. */ - def infoString(sym: Symbol, base: Type, showLocation: Boolean): String = atPhase(Phases.typerPhase) { + def infoString(sym: Symbol, base: Type, showLocation: Boolean): String = val sym1 = sym.underlyingSymbol def info = base.memberInfo(sym1) val infoStr = @@ -147,7 +147,6 @@ object ErrorReporting { else if sym1.isTerm then i" of type $info" else "" i"${if showLocation then sym1.showLocated else sym1}$infoStr" - } def infoStringWithLocation(sym: Symbol, base: Type) = infoString(sym, base, showLocation = true) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 52b210dd17ea..5ccd77b6e94a 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -336,10 +336,21 @@ object RefChecks { next == other || isInheritedAccessor(next, other) } + /** Detect any param section where params in last position do not agree isRepeatedParam. + */ def incompatibleRepeatedParam(member: Symbol, other: Symbol): Boolean = - member.is(Method, butNot = JavaDefined) && other.is(Method, butNot = JavaDefined) && atPhase(typerPhase) { - member.info.paramInfoss.nestedZipExists(other.info.paramInfoss)(_.isRepeatedParam != _.isRepeatedParam) - } + def loop(mParamInfoss: List[List[Type]], oParamInfoss: List[List[Type]]): Boolean = + mParamInfoss match + case Nil => false + case h :: t => + oParamInfoss match + case Nil => false + case h2 :: t2 => h.nonEmpty && h2.nonEmpty && h.last.isRepeatedParam != h2.last.isRepeatedParam + || loop(t, t2) + member.is(Method, butNot = JavaDefined) + && other.is(Method, butNot = JavaDefined) + && atPhase(typerPhase): + loop(member.info.paramInfoss, other.info.paramInfoss) /* Check that all conditions for overriding `other` by `member` * of class `clazz` are met. @@ -502,7 +513,7 @@ object RefChecks { else if member.is(Exported) then overrideError("cannot override since it comes from an export") else if incompatibleRepeatedParam(member, other) then - overrideError("cannot override because erased signatures conflict in repeated parameter") + report.error(DoubleDefinition(member, other, clazz), member.srcPos) else overrideError("needs `override` modifier") else if (other.is(AbsOverride) && other.isIncompleteIn(clazz) && !member.is(AbsOverride)) diff --git a/tests/neg/override-scala2-macro.check b/tests/neg/override-scala2-macro.check index fadf3a003402..ff5e478342dd 100644 --- a/tests/neg/override-scala2-macro.check +++ b/tests/neg/override-scala2-macro.check @@ -1,5 +1,5 @@ -- [E164] Declaration Error: tests/neg/override-scala2-macro.scala:2:22 ------------------------------------------------ 2 | override inline def f[A >: Any](args: A*): String = ??? // error | ^ - |error overriding method f in class StringContext of type [A >: Any](args: A*): String; - | method f of type [A >: Any](args: A*): String cannot be used here - only Scala-2 macros can override Scala-2 macros + |error overriding method f in class StringContext of type [A >: Any](args: Seq[A]): String; + | method f of type [A >: Any](args: Seq[A]): String cannot be used here - only Scala-2 macros can override Scala-2 macros