summaryrefslogtreecommitdiff
path: root/src/compiler
diff options
context:
space:
mode:
authorPaul Phillips <paulp@improving.org>2011-07-29 15:38:50 +0000
committerPaul Phillips <paulp@improving.org>2011-07-29 15:38:50 +0000
commit4e488a60594664046c3449e1aa2239adca7a012e (patch)
treee02baffa12f57600c28e0109adf66e4ac0a00920 /src/compiler
parentcda84e7f2145673e4e14cb886598fe19cd48585a (diff)
downloadscala-4e488a60594664046c3449e1aa2239adca7a012e.tar.gz
scala-4e488a60594664046c3449e1aa2239adca7a012e.tar.bz2
scala-4e488a60594664046c3449e1aa2239adca7a012e.zip
Added two new compiler options:
-Ywarn-adapted-args // also included in -Xlint -Yno-adapted-args The former warns when a () is inserted or an argument list is implicitly tupled. The latter errors under the same conditions. Using these options I found several bugs in the distribution which would otherwise be nearly impossible to spot. These bugs were innocuous (I think) but similar bugs could easily be (and have been) otherwise. Certain particularly threatening scenarios are at minimum warned about regardless of options given. Closes SI-4851, no review.
Diffstat (limited to 'src/compiler')
-rw-r--r--src/compiler/scala/reflect/internal/Definitions.scala5
-rw-r--r--src/compiler/scala/reflect/internal/Symbols.scala5
-rw-r--r--src/compiler/scala/tools/nsc/CompileServer.scala3
-rw-r--r--src/compiler/scala/tools/nsc/ast/parser/Parsers.scala5
-rw-r--r--src/compiler/scala/tools/nsc/interpreter/ByteCode.scala2
-rw-r--r--src/compiler/scala/tools/nsc/settings/ScalaSettings.scala3
-rw-r--r--src/compiler/scala/tools/nsc/settings/Warnings.scala4
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Adaptations.scala82
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Modes.scala3
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Typers.scala17
-rw-r--r--src/compiler/scala/tools/reflect/Shield.scala2
11 files changed, 115 insertions, 16 deletions
diff --git a/src/compiler/scala/reflect/internal/Definitions.scala b/src/compiler/scala/reflect/internal/Definitions.scala
index 00d2c232a0..857d7470d4 100644
--- a/src/compiler/scala/reflect/internal/Definitions.scala
+++ b/src/compiler/scala/reflect/internal/Definitions.scala
@@ -250,6 +250,8 @@ trait Definitions extends reflect.api.StandardDefinitions {
def scalaRuntimeSameElements = getMember(ScalaRunTimeModule, nme.sameElements)
// classes with special meanings
+ lazy val StringAddClass = getClass("scala.runtime.StringAdd")
+ lazy val StringAdd_+ = getMember(StringAddClass, nme.PLUS)
lazy val NotNullClass = getClass("scala.NotNull")
lazy val DelayedInitClass = getClass("scala.DelayedInit")
def delayedInitMethod = getMember(DelayedInitClass, nme.delayedInit)
@@ -363,6 +365,9 @@ trait Definitions extends reflect.api.StandardDefinitions {
lazy val NoneModule: Symbol = getModule("scala.None")
lazy val SomeModule: Symbol = getModule("scala.Some")
+ // The given symbol represents either String.+ or StringAdd.+
+ def isStringAddition(sym: Symbol) = sym == String_+ || sym == StringAdd_+
+
def isOptionType(tp: Type) = cond(tp.normalize) { case TypeRef(_, OptionClass, List(_)) => true }
def isSomeType(tp: Type) = cond(tp.normalize) { case TypeRef(_, SomeClass, List(_)) => true }
def isNoneType(tp: Type) = cond(tp.normalize) { case TypeRef(_, NoneModule, List(_)) => true }
diff --git a/src/compiler/scala/reflect/internal/Symbols.scala b/src/compiler/scala/reflect/internal/Symbols.scala
index af6b561699..5f2e060d6d 100644
--- a/src/compiler/scala/reflect/internal/Symbols.scala
+++ b/src/compiler/scala/reflect/internal/Symbols.scala
@@ -1804,6 +1804,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
case s => " in " + s
}
def fullLocationString = toString + locationString
+ def signatureString = if (hasRawInfo) infoString(rawInfo) else "<_>"
/** String representation of symbol's definition following its name */
final def infoString(tp: Type): String = {
@@ -1866,9 +1867,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
def defString = compose(
defaultFlagString,
keyString,
- varianceString + nameString + (
- if (hasRawInfo) infoString(rawInfo) else "<_>"
- )
+ varianceString + nameString + signatureString
)
/** Concatenate strings separated by spaces */
diff --git a/src/compiler/scala/tools/nsc/CompileServer.scala b/src/compiler/scala/tools/nsc/CompileServer.scala
index e72bc705c1..c23a671ef8 100644
--- a/src/compiler/scala/tools/nsc/CompileServer.scala
+++ b/src/compiler/scala/tools/nsc/CompileServer.scala
@@ -87,8 +87,7 @@ class StandardCompileServer extends SocketServer {
val input = in.readLine()
def fscError(msg: String): Unit = out println (
- FakePos("fsc"),
- msg + "\n fsc -help gives more information"
+ FakePos("fsc") + msg + "\n fsc -help gives more information"
)
if (input == null || password != guessedPassword)
return
diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
index 3ea091c394..cf2814165a 100644
--- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
+++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
@@ -2589,9 +2589,12 @@ self =>
*/
def templateParents(isTrait: Boolean): (List[Tree], List[List[Tree]]) = {
val parents = new ListBuffer[Tree] += startAnnotType()
- val argss =
+ val argss = (
+ // TODO: the insertion of List(Nil) here is where "new Foo" becomes
+ // indistinguishable from "new Foo()".
if (in.token == LPAREN && !isTrait) multipleArgumentExprs()
else List(Nil)
+ )
while (in.token == WITH) {
in.nextToken()
diff --git a/src/compiler/scala/tools/nsc/interpreter/ByteCode.scala b/src/compiler/scala/tools/nsc/interpreter/ByteCode.scala
index ece80af272..90d8fbb356 100644
--- a/src/compiler/scala/tools/nsc/interpreter/ByteCode.scala
+++ b/src/compiler/scala/tools/nsc/interpreter/ByteCode.scala
@@ -18,7 +18,7 @@ object ByteCode {
*/
private lazy val DECODER: Option[AnyRef] =
for (clazz <- getSystemLoader.tryToLoadClass[AnyRef]("scala.tools.scalap.Decode$")) yield
- clazz.getField(MODULE_INSTANCE_NAME).get()
+ clazz.getField(MODULE_INSTANCE_NAME).get(null)
private def decoderMethod(name: String, args: JClass*): Option[reflect.Method] = {
for (decoder <- DECODER ; m <- Option(decoder.getClass.getMethod(name, args: _*))) yield m
diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
index 838dd7404f..6325fd6eb4 100644
--- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
@@ -59,7 +59,7 @@ trait ScalaSettings extends AbsScalaSettings
val future = BooleanSetting ("-Xfuture", "Turn on future language features.")
val genPhaseGraph = StringSetting ("-Xgenerate-phase-graph", "file", "Generate the phase graphs (outputs .dot files) to fileX.dot.", "")
val XlogImplicits = BooleanSetting ("-Xlog-implicits", "Show more detail on why some implicits are not applicable.")
- val maxClassfileName = IntSetting ("-Xmax-classfile-name", "Maximum filename length for generated classes", 255, Some(72, 255), _ => None)
+ val maxClassfileName = IntSetting ("-Xmax-classfile-name", "Maximum filename length for generated classes", 255, Some(72, 255), _ => None)
val Xmigration28 = BooleanSetting ("-Xmigration", "Warn about constructs whose behavior may have changed between 2.7 and 2.8.")
val nouescape = BooleanSetting ("-Xno-uescape", "Disable handling of \\u unicode escapes.")
val Xnojline = BooleanSetting ("-Xnojline", "Do not use JLine for editing.")
@@ -117,6 +117,7 @@ trait ScalaSettings extends AbsScalaSettings
val Ynogenericsig = BooleanSetting ("-Yno-generic-signatures", "Suppress generation of generic signatures for Java.")
val noimports = BooleanSetting ("-Yno-imports", "Compile without importing scala.*, java.lang.*, or Predef.")
val nopredef = BooleanSetting ("-Yno-predef", "Compile without importing Predef.")
+ val noAdaptedArgs = BooleanSetting ("-Yno-adapted-args", "Do not adapt an argument list (either by inserting () or creating a tuple) to match the receiver.")
val Yprofile = PhasesSetting ("-Yprofile", "(Requires jvm -agentpath to contain yjgpagent) Profile CPU usage of given phases.")
val YprofileMem = BooleanSetting ("-Yprofile-memory", "Profile memory, get heap snapshot after each compiler run (requires yjpagent, see above).")
val YprofileClass = StringSetting ("-Yprofile-class", "class", "Name of profiler class.", "scala.tools.util.YourkitProfiling")
diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala
index 0d42fa503d..b2e3d51ad4 100644
--- a/src/compiler/scala/tools/nsc/settings/Warnings.scala
+++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala
@@ -28,7 +28,8 @@ trait Warnings {
warnDeadCode,
warnInaccessible,
warnNullaryOverride,
- warnNullaryUnit
+ warnNullaryUnit,
+ warnAdaptedArgs
)
// Warning groups.
@@ -43,6 +44,7 @@ trait Warnings {
// Individual warnings.
val warnSelectNullable = BooleanSetting ("-Xcheck-null", "Warn upon selection of nullable reference.")
+ val warnAdaptedArgs = BooleanSetting ("-Ywarn-adapted-args", "Warn if an argument list is modified to match the receiver.")
val warnDeadCode = BooleanSetting ("-Ywarn-dead-code", "Warn when dead code is identified.")
val warnValueDiscard = BooleanSetting ("-Ywarn-value-discard", "Warn when non-Unit expression results are unused.")
val warnNumericWiden = BooleanSetting ("-Ywarn-numeric-widen", "Warn when numerics are widened.")
diff --git a/src/compiler/scala/tools/nsc/typechecker/Adaptations.scala b/src/compiler/scala/tools/nsc/typechecker/Adaptations.scala
new file mode 100644
index 0000000000..3dee4650de
--- /dev/null
+++ b/src/compiler/scala/tools/nsc/typechecker/Adaptations.scala
@@ -0,0 +1,82 @@
+/* NSC -- new Scala compiler
+ * Copyright 2005-2011 LAMP/EPFL
+ * @author Paul Phillips
+ */
+
+package scala.tools.nsc
+package typechecker
+
+/** This trait provides logic for assessing the validity of argument
+ * adaptations, such as tupling, unit-insertion, widening, etc. Such
+ * logic is spread around the compiler, without much ability on the
+ * part of the user to tighten the potentially dangerous bits.
+ *
+ * TODO: unifying/consolidating said logic under consistent management.
+ *
+ * @author Paul Phillips
+ */
+trait Adaptations {
+ self: Analyzer =>
+
+ import global._
+ import definitions._
+
+ trait Adaptation {
+ self: Typer =>
+
+ def checkValidAdaptation(t: Tree, args: List[Tree]): Boolean = {
+ def applyArg = t match {
+ case Apply(_, arg :: Nil) => arg
+ case _ => EmptyTree
+ }
+ def callString = (
+ ( if (t.symbol.isConstructor) "new " else "" ) +
+ ( t.symbol.owner.decodedName ) +
+ ( if (t.symbol.isConstructor || t.symbol.name == nme.apply) "" else "." + t.symbol.decodedName )
+ )
+ def sigString = t.symbol.owner.decodedName + (
+ if (t.symbol.isConstructor) t.symbol.signatureString
+ else "." + t.symbol.decodedName + t.symbol.signatureString
+ )
+ def givenString = if (args.isEmpty) "<none>" else args.mkString(", ")
+ def adaptedArgs = if (args.isEmpty) "(): Unit" else args.mkString("(", ", ", "): " + applyArg.tpe)
+
+ def adaptWarning(msg: String) = context.warning(t.pos, msg +
+ "\n signature: " + sigString +
+ "\n given arguments: " + givenString +
+ "\n after adaptation: " + callString + "(" + adaptedArgs + ")"
+ )
+ // A one-argument method accepting Object (which may look like "Any"
+ // at this point if the class is java defined) is a "leaky target" for
+ // which we should be especially reluctant to insert () or auto-tuple.
+ def isLeakyTarget = {
+ val oneArgObject = t.symbol.paramss match {
+ case (param :: Nil) :: Nil => ObjectClass isSubClass param.tpe.typeSymbol
+ case _ => false
+ }
+ // Unfortunately various "universal" methods and the manner in which
+ // they are used limits our ability to enforce anything sensible until
+ // an opt-in compiler option is given.
+ oneArgObject && !(
+ isStringAddition(t.symbol)
+ || t.symbol.name == nme.equals_
+ || t.symbol.name == nme.EQ
+ || t.symbol.name == nme.NE
+ )
+ }
+
+ if (settings.noAdaptedArgs.value)
+ adaptWarning("No automatic adaptation here: use explicit parentheses.")
+ else if (settings.warnAdaptedArgs.value)
+ adaptWarning(
+ if (args.isEmpty) "Adapting argument list by inserting (): " + (
+ if (isLeakyTarget) "leaky (Object-receiving) target makes this especially dangerous."
+ else "this is unlikely to be what you want."
+ )
+ else "Adapting argument list by creating a " + args.size + "-tuple: this may not be what you want."
+ )
+
+ !settings.noAdaptedArgs.value
+ }
+ }
+}
diff --git a/src/compiler/scala/tools/nsc/typechecker/Modes.scala b/src/compiler/scala/tools/nsc/typechecker/Modes.scala
index ad4be4662c..48068b58d4 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Modes.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Modes.scala
@@ -106,6 +106,9 @@ trait Modes {
final def inPolyMode(mode: Int) = (mode & POLYmode) != 0
final def inPatternMode(mode: Int) = (mode & PATTERNmode) != 0
+ final def inExprModeButNot(mode: Int, prohibited: Int) =
+ (mode & (EXPRmode | prohibited)) == EXPRmode
+
/** Translates a mask of mode flags into something readable.
*/
private val modeNameMap = Map[Int, String](
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 13e0f13d4c..4125ab0285 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -27,7 +27,7 @@ import scala.tools.util.StringOps.{ countAsString, countElementsAsString }
* @author Martin Odersky
* @version 1.0
*/
-trait Typers extends Modes {
+trait Typers extends Modes with Adaptations {
self: Analyzer =>
import global._
@@ -77,7 +77,7 @@ trait Typers extends Modes {
// that are turned private by typedBlock
private final val SYNTHETIC_PRIVATE = TRANS_FLAG
- abstract class Typer(context0: Context) extends TyperDiagnostics {
+ abstract class Typer(context0: Context) extends TyperDiagnostics with Adaptation {
import context0.unit
import typeDebug.{ ptTree, ptBlock, ptLine }
@@ -882,7 +882,7 @@ trait Typers extends Modes {
typed(atPos(tree.pos)(Select(qual, nme.apply)), mode, pt)
} else if (!context.undetparams.isEmpty && !inPolyMode(mode)) { // (9)
assert(!inHKMode(mode)) //@M
- if ((mode & (EXPRmode | FUNmode)) == EXPRmode && (pt.typeSymbol == UnitClass))
+ if (inExprModeButNot(mode, FUNmode) && pt.typeSymbol == UnitClass)
instantiateExpectingUnit(tree, mode)
else
instantiate(tree, mode, pt)
@@ -898,7 +898,7 @@ trait Typers extends Modes {
val tree1 = constfold(tree, pt) // (10) (11)
if (tree1.tpe <:< pt) adapt(tree1, mode, pt, original)
else {
- if ((mode & (EXPRmode | FUNmode)) == EXPRmode) {
+ if (inExprModeButNot(mode, FUNmode)) {
pt.normalize match {
case TypeRef(_, sym, _) =>
// note: was if (pt.typeSymbol == UnitClass) but this leads to a potentially
@@ -2346,8 +2346,13 @@ trait Typers extends Modes {
val savedUndetparams = context.undetparams
silent(_.doTypedApply(tree, fun, tupleArgs, mode, pt)) match {
case t: Tree =>
-// println("tuple conversion to "+t+" for "+mt)//DEBUG
- Some(t)
+ // Depending on user options, may warn or error here if
+ // a Unit or tuple was inserted.
+ Some(t) filter (tupledTree =>
+ !inExprModeButNot(mode, FUNmode)
+ || tupledTree.symbol == null
+ || checkValidAdaptation(tupledTree, args)
+ )
case ex =>
context.undetparams = savedUndetparams
None
diff --git a/src/compiler/scala/tools/reflect/Shield.scala b/src/compiler/scala/tools/reflect/Shield.scala
index 19730791b9..f9c7e54454 100644
--- a/src/compiler/scala/tools/reflect/Shield.scala
+++ b/src/compiler/scala/tools/reflect/Shield.scala
@@ -36,7 +36,7 @@ trait Shield {
def method(name: String, arity: Int) = uniqueMethod(name, arity)
def field(name: String) = clazz getField name
- def matchingMethods(name: String, arity: Int) = methods filter (m => nameAndArity(m) == (name, arity))
+ def matchingMethods(name: String, arity: Int) = methods filter (m => nameAndArity(m) == ((name, arity)))
def uniqueMethod(name: String, arity: Int) = matchingMethods(name, arity) match {
case List(x) => x
case _ => onError("No unique match for " + name)