diff options
author | Paul Phillips <paulp@improving.org> | 2011-08-06 19:34:49 +0000 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2011-08-06 19:34:49 +0000 |
commit | 990fa046e6bbe95d5def208b38fead77d2437f9f (patch) | |
tree | caed0b83c9d4220f4ebe48ab844a8c41884df45d | |
parent | ead69ed24557ff42966a2bd5f71b6434ac5343b6 (diff) | |
download | scala-990fa046e6bbe95d5def208b38fead77d2437f9f.tar.gz scala-990fa046e6bbe95d5def208b38fead77d2437f9f.tar.bz2 scala-990fa046e6bbe95d5def208b38fead77d2437f9f.zip |
Fixed bug in the disambiguation of f(foo='bar')...
Fixed bug in the disambiguation of f(foo='bar') style method calls in
the presence of overloading, parameterization, and by-name arguments.
Took the opportunity to clean things up a little bit. Closes SI-4592,
review by rytz.
-rw-r--r-- | src/compiler/scala/reflect/internal/AnnotationInfos.scala | 24 | ||||
-rw-r--r-- | src/compiler/scala/reflect/internal/Symbols.scala | 2 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Infer.scala | 92 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala | 17 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Typers.scala | 6 | ||||
-rw-r--r-- | test/files/run/bug4592.check | 3 | ||||
-rw-r--r-- | test/files/run/bug4592.scala | 10 |
7 files changed, 101 insertions, 53 deletions
diff --git a/src/compiler/scala/reflect/internal/AnnotationInfos.scala b/src/compiler/scala/reflect/internal/AnnotationInfos.scala index 62b27c2daf..dcabdf0a27 100644 --- a/src/compiler/scala/reflect/internal/AnnotationInfos.scala +++ b/src/compiler/scala/reflect/internal/AnnotationInfos.scala @@ -118,17 +118,21 @@ trait AnnotationInfos extends api.AnnotationInfos { self: SymbolTable => AnnotationInfo(atp, args.map(subs(_)), assocs).setPos(pos) } - // !!! when annotation arguments are not literal strings, but any sort of - // assembly of strings, there is a fair chance they will turn up here not as + def stringArg(index: Int) = constantAtIndex(index) map (_.stringValue) + def intArg(index: Int) = constantAtIndex(index) map (_.intValue) + def symbolArg(index: Int) = argAtIndex(index) collect { + case Apply(fun, Literal(str) :: Nil) if fun.symbol == definitions.Symbol_apply => + newTermName(str.stringValue) + } + + // !!! when annotation arguments are not literals, but any sort of + // expression, there is a fair chance they will turn up here not as // Literal(const) but some arbitrary AST. - def stringArg(index: Int): Option[String] = if(args.size > index) Some(args(index) match { - case Literal(const) => const.stringValue - case x => x.toString // should not be necessary, but better than silently ignoring an issue - }) else None - - def intArg(index: Int): Option[Int] = if(args.size > index) Some(args(index)) collect { - case Literal(Constant(x: Int)) => x - } else None + def constantAtIndex(index: Int): Option[Constant] = + argAtIndex(index) collect { case Literal(x) => x } + + def argAtIndex(index: Int): Option[Tree] = + if (index < args.size) Some(args(index)) else None } object AnnotationInfo extends AnnotationInfoExtractor diff --git a/src/compiler/scala/reflect/internal/Symbols.scala b/src/compiler/scala/reflect/internal/Symbols.scala index 3e80f1cdee..4789d69c69 100644 --- a/src/compiler/scala/reflect/internal/Symbols.scala +++ b/src/compiler/scala/reflect/internal/Symbols.scala @@ -437,6 +437,8 @@ trait Symbols extends api.Symbols { self: SymbolTable => def hasBridgeAnnotation = hasAnnotation(BridgeClass) def deprecationMessage = getAnnotation(DeprecatedAttr) flatMap (_ stringArg 0) def deprecationVersion = getAnnotation(DeprecatedAttr) flatMap (_ stringArg 1) + def deprecatedParamName = getAnnotation(DeprecatedNameAttr) flatMap (_ symbolArg 0) + // !!! when annotation arguments are not literal strings, but any sort of // assembly of strings, there is a fair chance they will turn up here not as // Literal(const) but some arbitrary AST. However nothing in the compiler diff --git a/src/compiler/scala/tools/nsc/typechecker/Infer.scala b/src/compiler/scala/tools/nsc/typechecker/Infer.scala index 7305cef34f..495594da5e 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Infer.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Infer.scala @@ -755,7 +755,8 @@ trait Infer { val argtpes1 = argtpes map { case NamedType(name, tp) => // a named argument var res = tp - val pos = params.indexWhere(p => (p.name == name || deprecatedName(p) == Some(name)) && !p.isSynthetic) + val pos = params.indexWhere(p => paramMatchesName(p, name) && !p.isSynthetic) + if (pos == -1) { if (positionalAllowed) { // treat assignment as positional argument argPos(index) = index @@ -1634,6 +1635,54 @@ trait Infer { } } + @inline private def wrapTypeError(expr: => Boolean): Boolean = + try expr + catch { case _: TypeError => false } + + // Checks against the name of the parameter and also any @deprecatedName. + private def paramMatchesName(param: Symbol, name: Name) = + param.name == name || param.deprecatedParamName.exists(_ == name) + + // Check the first parameter list the same way. + private def methodMatchesName(method: Symbol, name: Name) = method.paramss match { + case ps :: _ => ps exists (p => paramMatchesName(p, name)) + case _ => false + } + + private def resolveOverloadedMethod(argtpes: List[Type], eligible: List[Symbol]) = { + // If there are any foo=bar style arguments, and any of the overloaded + // methods has a parameter named `foo`, then only those methods are considered. + val namesOfArgs = argtpes collect { case NamedType(name, _) => name } + val namesMatch = ( + if (namesOfArgs.isEmpty) Nil + else eligible filter { m => + namesOfArgs forall { name => + methodMatchesName(m, name) + } + } + ) + + if (namesMatch.nonEmpty) namesMatch + else if (eligible.isEmpty || eligible.tail.isEmpty) eligible + else eligible filter { alt => + // for functional values, the `apply` method might be overloaded + val mtypes = followApply(alt.tpe) match { + case OverloadedType(_, alts) => alts map (_.tpe) + case t => List(t) + } + // Drop those that use a default; keep those that use vararg/tupling conversion. + mtypes exists (t => + !t.typeSymbol.hasDefaultFlag && { + compareLengths(t.params, argtpes) < 0 || // tupling (*) + hasExactlyNumParams(t, argtpes.length) // same nb or vararg + } + ) + // (*) more arguments than parameters, but still applicable: tupling conversion works. + // todo: should not return "false" when paramTypes = (Unit) no argument is given + // (tupling would work) + } + } + /** Assign <code>tree</code> the type of an alternative which is applicable * to <code>argtpes</code>, and whose result type is compatible with `pt`. * If several applicable alternatives exist, drop the alternatives which use @@ -1656,40 +1705,21 @@ trait Infer { debuglog("infer method alt "+ tree.symbol +" with alternatives "+ (alts map pre.memberType) +", argtpes = "+ argtpes +", pt = "+ pt) - var allApplicable = alts filter (alt => - // TODO: this will need to be re-written once we substitute throwing exceptions - // with generating error trees. We wrap this applicability in try/catch because of #4457. - try {isApplicable(undetparams, followApply(pre.memberType(alt)), argtpes, pt)} catch {case _: TypeError => false}) - - //log("applicable: "+ (allApplicable map pre.memberType)) - - if (varArgsOnly) - allApplicable = allApplicable filter (alt => isVarArgsList(alt.tpe.params)) - - // if there are multiple, drop those that use a default - // (keep those that use vararg / tupling conversion) - val applicable = - if (allApplicable.lengthCompare(1) <= 0) allApplicable - else allApplicable filter (alt => { - val mtypes = followApply(alt.tpe) match { - // for functional values, the `apply` method might be overloaded - case OverloadedType(_, alts) => alts map (_.tpe) - case t => List(t) - } - mtypes exists (t => - compareLengths(t.params, argtpes) < 0 || // tupling (*) - hasExactlyNumParams(t, argtpes.length) // same nb or vararg - ) - // (*) more arguments than parameters, but still applicable: tuplig conversion works. - // todo: should not return "false" when paramTypes = (Unit) no argument is given - // (tupling would work) - }) + val applicable = resolveOverloadedMethod(argtpes, { + alts filter { alt => + // TODO: this will need to be re-written once we substitute throwing exceptions + // with generating error trees. We wrap this applicability in try/catch because of #4457. + wrapTypeError(isApplicable(undetparams, followApply(pre.memberType(alt)), argtpes, pt)) && + (!varArgsOnly || isVarArgsList(alt.tpe.params)) + } + }) - def improves(sym1: Symbol, sym2: Symbol) = -// util.trace("improve "+sym1+sym1.locationString+" on "+sym2+sym2.locationString)( + def improves(sym1: Symbol, sym2: Symbol) = { + // util.trace("improve "+sym1+sym1.locationString+" on "+sym2+sym2.locationString) sym2 == NoSymbol || sym2.isError || sym2.hasAnnotation(BridgeClass) || isStrictlyMoreSpecific(followApply(pre.memberType(sym1)), followApply(pre.memberType(sym2)), sym1, sym2) + } val best = ((NoSymbol: Symbol) /: applicable) ((best, alt) => if (improves(alt, best)) alt else best) diff --git a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala index a5c5aa5320..bf7cc72fab 100644 --- a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala +++ b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala @@ -21,8 +21,13 @@ trait NamesDefaults { self: Analyzer => val defaultParametersOfMethod = perRunCaches.newWeakMap[Symbol, Set[Symbol]]() withDefaultValue Set() - case class NamedApplyInfo(qual: Option[Tree], targs: List[Tree], - vargss: List[List[Tree]], blockTyper: Typer) + case class NamedApplyInfo( + qual: Option[Tree], + targs: List[Tree], + vargss: List[List[Tree]], + blockTyper: Typer + ) { } + val noApplyInfo = NamedApplyInfo(None, Nil, Nil, null) def nameOf(arg: Tree) = arg match { @@ -558,17 +563,11 @@ trait NamesDefaults { self: Analyzer => val p = rest.head if (!p.isSynthetic) { if (p.name == name) return (i, None) - if (deprecatedName(p) == Some(name)) return (i, Some(p.name)) + if (p.deprecatedParamName == Some(name)) return (i, Some(p.name)) } i += 1 rest = rest.tail } (-1, None) } - - def deprecatedName(sym: Symbol): Option[Name] = - sym.getAnnotation(DeprecatedNameAttr).map(ann => (ann.args(0): @unchecked) match { - case Apply(fun, Literal(str) :: Nil) if (fun.symbol == Symbol_apply) => - newTermName(str.stringValue) - }) } diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index f305644837..7ae390a992 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1795,10 +1795,10 @@ trait Typers extends Modes with Adaptations { if (phase.id <= currentRun.typerPhase.id) { val allParams = meth.paramss.flatten for (p <- allParams) { - deprecatedName(p).foreach(n => { - if (allParams.exists(p1 => p1.name == n || (p != p1 && deprecatedName(p1) == Some(n)))) + for (n <- p.deprecatedParamName) { + if (allParams.exists(p1 => p1.name == n || (p != p1 && p1.deprecatedParamName.exists(_ == n)))) error(p.pos, "deprecated parameter name "+ n +" has to be distinct from any other parameter name (deprecated or not).") - }) + } } } diff --git a/test/files/run/bug4592.check b/test/files/run/bug4592.check new file mode 100644 index 0000000000..e133386482 --- /dev/null +++ b/test/files/run/bug4592.check @@ -0,0 +1,3 @@ +3.14 +3.14 +3.14 diff --git a/test/files/run/bug4592.scala b/test/files/run/bug4592.scala new file mode 100644 index 0000000000..d1666d84d7 --- /dev/null +++ b/test/files/run/bug4592.scala @@ -0,0 +1,10 @@ +object Test { + def repeat[T](count: Int = 1, x: Boolean = true)(thunk: => T) : T = (0 until count).map(_ => thunk).last + def repeat[T](thunk: => T) : T = repeat()(thunk) + + def main(args: Array[String]): Unit = { + println(repeat(3.14)) + println(repeat(count=5)(3.14)) + println(repeat(count=5,x=false)(3.14)) + } +} |