From 6634d82f74b71dd36ca1ecceea08b4ad79335da0 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 19 Aug 2013 10:23:11 +0200 Subject: SI-1980 A lint warning for by-name parameters in right assoc methods The desugaring of right associative calls happens in the parser. This eagerly evaluates the arguments (to preserve left-to-right evaluation order the arguments are evaluated before the qualifier). This is pretty surprising if the method being called has a by-name parameter in the first parameter section. This commit adds a warning under -Xlint when defining such a method. The relevent spec snippets: > SLS 4.6.1 says that call-by-name argument "is not evaluated at the point of function application, but instead is evaluated at each use within the function". > > But 6.12.3 offers: > "If op is right- associative, the same operation is interpreted as { val x=e1; e2.op(x ) }, where x is a fresh name." --- src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 14 ++++++++++++++ test/files/neg/t1980.check | 12 ++++++++++++ test/files/neg/t1980.flags | 1 + test/files/neg/t1980.scala | 9 +++++++++ 4 files changed, 36 insertions(+) create mode 100644 test/files/neg/t1980.check create mode 100644 test/files/neg/t1980.flags create mode 100644 test/files/neg/t1980.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 1b6963b598..333d867797 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1371,6 +1371,16 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans member.typeParams.map(_.info.bounds.hi.widen) foreach checkAccessibilityOfType } + private def checkByNameRightAssociativeDef(tree: DefDef) { + tree match { + case DefDef(_, name, _, params :: _, _, _) => + if (settings.lint && !treeInfo.isLeftAssoc(name.decodedName) && params.exists(p => isByName(p.symbol))) + unit.warning(tree.pos, + "by-name parameters will be evaluated eagerly when called as a right-associative infix operator. For more details, see SI-1980.") + case _ => + } + } + /** Check that a deprecated val or def does not override a * concrete, non-deprecated method. If it does, then * deprecation is meaningless. @@ -1594,6 +1604,10 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans if (!sym.isConstructor && !sym.isEffectivelyFinal && !sym.isSynthetic) checkAccessibilityOfReferencedTypes(tree) } + tree match { + case dd: DefDef => checkByNameRightAssociativeDef(dd) + case _ => + } tree case Template(parents, self, body) => diff --git a/test/files/neg/t1980.check b/test/files/neg/t1980.check new file mode 100644 index 0000000000..2fa27fa462 --- /dev/null +++ b/test/files/neg/t1980.check @@ -0,0 +1,12 @@ +t1980.scala:2: warning: by-name parameters will be evaluated eagerly when called as a right-associative infix operator. For more details, see SI-1980. + def op1_:(x: => Any) = () // warn + ^ +t1980.scala:3: warning: by-name parameters will be evaluated eagerly when called as a right-associative infix operator. For more details, see SI-1980. + def op2_:(x: Any, y: => Any) = () // warn + ^ +t1980.scala:4: warning: by-name parameters will be evaluated eagerly when called as a right-associative infix operator. For more details, see SI-1980. + def op3_:(x: Any, y: => Any)(a: Any) = () // warn + ^ +error: No warnings can be incurred under -Xfatal-warnings. +three warnings found +one error found diff --git a/test/files/neg/t1980.flags b/test/files/neg/t1980.flags new file mode 100644 index 0000000000..7949c2afa2 --- /dev/null +++ b/test/files/neg/t1980.flags @@ -0,0 +1 @@ +-Xlint -Xfatal-warnings diff --git a/test/files/neg/t1980.scala b/test/files/neg/t1980.scala new file mode 100644 index 0000000000..132865e694 --- /dev/null +++ b/test/files/neg/t1980.scala @@ -0,0 +1,9 @@ +object Test { + def op1_:(x: => Any) = () // warn + def op2_:(x: Any, y: => Any) = () // warn + def op3_:(x: Any, y: => Any)(a: Any) = () // warn + + def op4() = () // no warn + def op5(x: => Any) = () // no warn + def op6_:(x: Any)(a: => Any) = () // no warn +} -- cgit v1.2.3