Skip to content
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

Actually retract clashing synthetic apply/unapply #5846

Merged
merged 1 commit into from Apr 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/compiler/scala/tools/nsc/typechecker/Namers.scala
Expand Up @@ -691,8 +691,21 @@ trait Namers extends MethodSynthesis {

if (suppress) {
sym setInfo ErrorType
// There are two ways in which we exclude the symbol from being added in typedStats::addSynthetics,
// because we don't know when the completer runs with respect to this loop in addSynthetics
// for (sym <- scope)
// for (tree <- context.unit.synthetics.get(sym) if shouldAdd(sym)) {
// if (!sym.initialize.hasFlag(IS_ERROR))
// newStats += typedStat(tree)
// (1) If we're already in the loop, set the IS_ERROR flag and trigger the condition
// `sym.initialize.hasFlag(IS_ERROR)` in typedStats::addSynthetics,
// (2) Or, if we are not yet in the addSynthetics loop (and we're not going to emit an error anyway),
// we unlink the symbol from its scope.
sym setFlag IS_ERROR
Copy link
Member

@lrytz lrytz Apr 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • add companionContext.unit.synthetics -= sym?


// For good measure. Removing it from its owner's scope and setting the IS_ERROR flag is enough to exclude it from addSynthetics
companionContext.unit.synthetics -= sym

// Don't unlink in an error situation to generate less confusing error messages.
// Ideally, our error reporting would distinguish overloaded from recursive user-defined apply methods without signature,
// but this would require some form of partial-completion of method signatures, so that we can
Expand All @@ -702,7 +715,7 @@ trait Namers extends MethodSynthesis {
// I hesitate to provide more info, because it would involve a WildCard or something for its result type,
// which could upset other code paths)
if (!scopePartiallyCompleted)
companionContext.scope.unlink(sym)
companionContext.scope.unlink(sym) // (2)
}
}
}
Expand Down Expand Up @@ -770,7 +783,7 @@ trait Namers extends MethodSynthesis {
val completer =
if (sym hasFlag SYNTHETIC) {
if (name == nme.copy) copyMethodCompleter(tree)
else if (sym hasFlag CASE) applyUnapplyMethodCompleter(tree, context)
else if (settings.isScala212 && (sym hasFlag CASE)) applyUnapplyMethodCompleter(tree, context)
else completerOf(tree)
} else completerOf(tree)

Expand Down
6 changes: 5 additions & 1 deletion src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -3093,6 +3093,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
val scope = if (inBlock) context.scope else context.owner.info.decls
var newStats = new ListBuffer[Tree]
var moreToAdd = true
val retractErroneousSynthetics = settings.isScala212

while (moreToAdd) {
val initElems = scope.elems
// SI-5877 The decls of a package include decls of the package object. But we don't want to add
Expand All @@ -3101,7 +3103,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
inBlock || !context.isInPackageObject(sym, context.owner)
for (sym <- scope)
for (tree <- context.unit.synthetics get sym if shouldAdd(sym)) { // OPT: shouldAdd is usually true. Call it here, rather than in the outer loop
newStats += typedStat(tree) // might add even more synthetics to the scope
// if the completer set the IS_ERROR flag, retract the stat (currently only used by applyUnapplyMethodCompleter)
if (!(retractErroneousSynthetics && sym.initialize.hasFlag(IS_ERROR)))
newStats += typedStat(tree) // might add even more synthetics to the scope
context.unit.synthetics -= sym
}
// the type completer of a synthetic might add more synthetics. example: if the
Expand Down
1 change: 1 addition & 0 deletions test/files/neg/userdefined_apply.flags
@@ -0,0 +1 @@
-Xsource:2.12
1 change: 1 addition & 0 deletions test/files/pos/userdefined_apply.flags
@@ -0,0 +1 @@
-Xsource:2.12
1 change: 1 addition & 0 deletions test/files/pos/userdefined_apply_poly_overload.flags
@@ -0,0 +1 @@
-Xsource:2.12
1 change: 1 addition & 0 deletions test/files/run/t10261.flags
@@ -0,0 +1 @@
-Xsource:2.12
4 changes: 4 additions & 0 deletions test/files/run/t10261/Companion_1.scala
@@ -0,0 +1,4 @@
trait Companion[T] {
def parse(value: String): Option[T]
def apply(value: String): T = parse(value).get
}
14 changes: 14 additions & 0 deletions test/files/run/t10261/Test_2.scala
@@ -0,0 +1,14 @@
import scala.util.Try

object C extends Companion[C] {
def parse(v: String) = if (v.nonEmpty) Some(new C(v)) else None
}

case class C(value: String)

object Test {
def main(args: Array[String]): Unit = {
assert(Try{C("")}.isFailure, "Empty value should fail to parse") // check that parse is used to validate input
assert(C("a").value == "a", "Unexpected value")
}
}