Skip to content

Commit

Permalink
Repeated params must correspond in override (scala#16836)
Browse files Browse the repository at this point in the history
Refchecks runs after elimRepeated and did not
error on an attempt to override RepeatedParam with Seq.

Also show RepeatedParam in error message for double definition.

Fixes scala#12662

Resubmits scala#13248 which was opened
against old master.
  • Loading branch information
odersky authored Jun 26, 2023
2 parents bbb70cc + d6fd2c4 commit 9009f16
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 7 deletions.
31 changes: 24 additions & 7 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,22 @@ 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 =
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.
*/
Expand Down Expand Up @@ -425,10 +441,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
Expand Down Expand Up @@ -498,6 +512,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
report.error(DoubleDefinition(member, other, clazz), member.srcPos)
else
overrideError("needs `override` modifier")
else if (other.is(AbsOverride) && other.isIncompleteIn(clazz) && !member.is(AbsOverride))
Expand Down Expand Up @@ -878,6 +894,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
Expand Down Expand Up @@ -910,8 +927,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
Expand All @@ -935,7 +952,7 @@ object RefChecks {
}
member.resetFlag(Override)
member.resetFlag(AbsOverride)
}
end if
}

/** Check that we do not "override" anything with a private method
Expand Down
21 changes: 21 additions & 0 deletions tests/neg/overrides.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 9009f16

Please sign in to comment.