Skip to content

Commit a60178d

Browse files
committed
Restrict auto-tupling more
This was initially an attempt to drop auto-tupling altogether. But it became quickly annoying. The main problem is that it's unnatural to add another pair of parentheses to infix operations. Compare: (x, y) == z z == ((x, y)) // !yuck Same thing for cons: ((1, x)) :: ((2, y)) :: xs // double yuck! So at the very least we have to allow auto-tupling for arguments of infix operators. I went a little bit further and also allowed auto-tupling if the expected type is already a (some kind of) product or tuple of the right arity. The result is this commit. The language import `noAutoTupling` has been removed. The previous auto-tupling is still reported under Scala 2 mode, and there is -rewrite support to add the missing (...) automatically.
1 parent bbcd512 commit a60178d

File tree

23 files changed

+94
-96
lines changed

23 files changed

+94
-96
lines changed

compiler/src/dotty/tools/dotc/ast/Desugar.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1216,7 +1216,7 @@ object desugar {
12161216
*/
12171217
private object IdPattern {
12181218
def unapply(tree: Tree)(implicit ctx: Context): Option[VarInfo] = tree match {
1219-
case id: Ident => Some(id, TypeTree())
1219+
case id: Ident => Some((id, TypeTree()))
12201220
case Typed(id: Ident, tpt) => Some((id, tpt))
12211221
case _ => None
12221222
}

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
537537
object closure {
538538
def unapply(tree: Tree): Option[(List[Tree], Tree, Tree)] = tree match {
539539
case Block(_, expr) => unapply(expr)
540-
case Closure(env, meth, tpt) => Some(env, meth, tpt)
540+
case Closure(env, meth, tpt) => Some((env, meth, tpt))
541541
case Typed(expr, _) => unapply(expr)
542542
case _ => None
543543
}

compiler/src/dotty/tools/dotc/ast/tpd.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,8 +1009,8 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
10091009
/** An extractor that pulls out type arguments */
10101010
object MaybePoly {
10111011
def unapply(tree: Tree): Option[(Tree, List[Tree])] = tree match {
1012-
case TypeApply(tree, targs) => Some(tree, targs)
1013-
case _ => Some(tree, Nil)
1012+
case TypeApply(tree, targs) => Some((tree, targs))
1013+
case _ => Some((tree, Nil))
10141014
}
10151015
}
10161016

compiler/src/dotty/tools/dotc/core/Definitions.scala

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,6 @@ class Definitions {
436436
lazy val ArrayModuleType = ctx.requiredModuleRef("scala.Array")
437437
def ArrayModule(implicit ctx: Context) = ArrayModuleType.symbol.moduleClass.asClass
438438

439-
440439
lazy val UnitType: TypeRef = valueTypeRef("scala.Unit", BoxedUnitType, java.lang.Void.TYPE, UnitEnc, nme.specializedTypeNames.Void)
441440
def UnitClass(implicit ctx: Context) = UnitType.symbol.asClass
442441
def UnitModuleClass(implicit ctx: Context) = UnitType.symbol.asClass.linkedClass
@@ -776,7 +775,7 @@ class Definitions {
776775
if (isFunctionClass(tsym)) {
777776
val targs = ft.dealias.argInfos
778777
if (targs.isEmpty) None
779-
else Some(targs.init, targs.last, tsym.name.isImplicitFunction, tsym.name.isErasedFunction)
778+
else Some((targs.init, targs.last, tsym.name.isImplicitFunction, tsym.name.isErasedFunction))
780779
}
781780
else None
782781
}
@@ -821,8 +820,8 @@ class Definitions {
821820
case ArrayOf(elemtp) =>
822821
def recur(elemtp: Type): Option[(Type, Int)] = elemtp.dealias match {
823822
case TypeBounds(lo, hi) => recur(hi)
824-
case MultiArrayOf(finalElemTp, n) => Some(finalElemTp, n + 1)
825-
case _ => Some(elemtp, 1)
823+
case MultiArrayOf(finalElemTp, n) => Some((finalElemTp, n + 1))
824+
case _ => Some((elemtp, 1))
826825
}
827826
recur(elemtp)
828827
case _ =>
@@ -879,6 +878,18 @@ class Definitions {
879878
name.length > prefix.length &&
880879
name.drop(prefix.length).forall(_.isDigit))
881880

881+
def arity(cls: Symbol, prefix: String): Int =
882+
scalaClassName(cls).applySimple(-1) { name =>
883+
if (name.startsWith(prefix)) {
884+
val digits = name.drop(prefix.length)
885+
if (!digits.isEmpty && digits.forall(_.isDigit))
886+
try digits.toString.toInt
887+
catch { case ex: NumberFormatException => -1 }
888+
else -1
889+
}
890+
else -1
891+
}
892+
882893
def isBottomClass(cls: Symbol) =
883894
cls == NothingClass || cls == NullClass
884895
def isBottomType(tp: Type) =

compiler/src/dotty/tools/dotc/core/NameOps.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ object NameOps {
5858
case _ => false
5959
}
6060

61+
def applySimple[T](default: T)(f: SimpleName => T): T = name match {
62+
case name: SimpleName => f(name)
63+
case name: TypeName => name.toTermName.applySimple(default)(f)
64+
case _ => default
65+
}
66+
6167
def likeSpaced(n: PreName): N =
6268
(if (name.isTermName) n.toTermName else n.toTypeName).asInstanceOf[N]
6369

@@ -81,6 +87,11 @@ object NameOps {
8187
}
8288
}
8389

90+
def isOpName = name match {
91+
case name: SimpleName => NameTransformer.encode(name) != name
92+
case _ => false
93+
}
94+
8495
def isOpAssignmentName: Boolean = name match {
8596
case raw.NE | raw.LE | raw.GE | EMPTY =>
8697
false

compiler/src/dotty/tools/dotc/core/TypeOps.scala

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import util.Property
1717
import collection.mutable
1818
import ast.tpd._
1919
import reporting.trace
20-
import reporting.diagnostic.Message
20+
import reporting.diagnostic.{Message, NoExplanation}
2121

2222
trait TypeOps { this: Context => // TODO: Make standalone object.
2323

@@ -313,10 +313,6 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
313313
hasImport(ctx.withPhase(ctx.typerPhase)) || hasOption
314314
}
315315

316-
/** Is auto-tupling enabled? */
317-
def canAutoTuple =
318-
!featureEnabled(defn.LanguageModuleClass, nme.noAutoTupling)
319-
320316
def scala2Mode =
321317
featureEnabled(defn.LanguageModuleClass, nme.Scala2)
322318

@@ -325,7 +321,8 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
325321

326322
def testScala2Mode(msg: => Message, pos: Position, rewrite: => Unit = ()) = {
327323
if (scala2Mode) {
328-
migrationWarning(msg, pos)
324+
migrationWarning(
325+
new NoExplanation(msg.msg ++ "\nThis can be fixed automatically using -rewrite"), pos)
329326
rewrite
330327
}
331328
scala2Mode

compiler/src/dotty/tools/dotc/typer/Applications.scala

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -980,10 +980,12 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
980980
val (regularArgs, varArgs) = args.splitAt(argTypes.length - 1)
981981
regularArgs :+ untpd.SeqLiteral(varArgs, untpd.TypeTree()).withPos(tree.pos)
982982
}
983-
else if (argTypes.lengthCompare(1) == 0 && args.lengthCompare(1) > 0 && ctx.canAutoTuple)
984-
untpd.Tuple(args) :: Nil
985-
else
986-
args
983+
else argTypes match {
984+
case argType :: Nil if args.lengthCompare(1) > 0 && supportsAutoTupling(argType, args) =>
985+
untpd.Tuple(args) :: Nil
986+
case _ =>
987+
args
988+
}
987989
if (argTypes.length != bunchedArgs.length) {
988990
ctx.error(UnapplyInvalidNumberOfArguments(qual, argTypes), tree.pos)
989991
argTypes = argTypes.take(args.length) ++

compiler/src/dotty/tools/dotc/typer/Typer.scala

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,6 +2032,45 @@ class Typer extends Namer
20322032
}
20332033
}
20342034

2035+
def supportsAutoTupling(tp: Type, args: List[untpd.Tree])(implicit ctx: Context): Boolean = {
2036+
def test(tp: Type): Boolean = tp match {
2037+
case tp: TypeRef =>
2038+
val sym = tp.symbol
2039+
defn.isTupleClass(sym) && defn.arity(sym, str.Tuple) == args.length ||
2040+
defn.isProductClass(sym) && defn.arity(sym, str.Product) == args.length ||
2041+
!sym.isClass && test(tp.underlying)
2042+
case tp: TypeParamRef =>
2043+
val bounds = ctx.typeComparer.bounds(tp)
2044+
test(bounds.lo) || test(bounds.hi)
2045+
case tp: TypeProxy =>
2046+
test(tp.underlying)
2047+
case _ =>
2048+
false
2049+
}
2050+
test(tp) || {
2051+
val pos = Position(args.map(_.pos.start).min, args.map(_.pos.end).max)
2052+
ctx.testScala2Mode(
2053+
i"""auto-tupling is no longer supported in this case,
2054+
|arguments now need to be enclosed in parentheses (...).""",
2055+
pos, { patch(pos.startPos, "("); patch(pos.endPos, ")") })
2056+
}
2057+
}
2058+
2059+
def takesAutoTupledArgs(tp: Type, sym: Symbol, args: List[untpd.Tree])(implicit ctx: Context): Boolean = tp match {
2060+
case tp: MethodicType =>
2061+
tp.firstParamTypes match {
2062+
case ptype :: Nil =>
2063+
!ptype.isRepeatedParam && {
2064+
sym.name.isOpName || supportsAutoTupling(ptype, args)
2065+
}
2066+
case _ => false
2067+
}
2068+
case tp: TermRef =>
2069+
tp.denot.alternatives.forall(alt => takesAutoTupledArgs(alt.info, alt.symbol, args))
2070+
case _ =>
2071+
false
2072+
}
2073+
20352074
/** Perform the following adaptations of expression, pattern or type `tree` wrt to
20362075
* given prototype `pt`:
20372076
* (1) Resolve overloading
@@ -2119,21 +2158,9 @@ class Typer extends Namer
21192158
}
21202159
}
21212160

2122-
def isUnary(tp: Type): Boolean = tp match {
2123-
case tp: MethodicType =>
2124-
tp.firstParamTypes match {
2125-
case ptype :: Nil => !ptype.isRepeatedParam
2126-
case _ => false
2127-
}
2128-
case tp: TermRef =>
2129-
tp.denot.alternatives.forall(alt => isUnary(alt.info))
2130-
case _ =>
2131-
false
2132-
}
2133-
21342161
def adaptToArgs(wtp: Type, pt: FunProto): Tree = wtp match {
21352162
case _: MethodOrPoly =>
2136-
if (pt.args.lengthCompare(1) > 0 && isUnary(wtp) && ctx.canAutoTuple)
2163+
if (pt.args.lengthCompare(1) > 0 && takesAutoTupledArgs(wtp, tree.symbol, pt.args))
21372164
adapt(tree, pt.tupled, locked)
21382165
else
21392166
tree
@@ -2499,10 +2526,10 @@ class Typer extends Namer
24992526
case tp: FlexType =>
25002527
ensureReported(tp)
25012528
tree
2502-
case ref: TermRef =>
2529+
case ref: TermRef => // this case can happen in case tree.tpe is overloaded
25032530
pt match {
25042531
case pt: FunProto
2505-
if pt.args.lengthCompare(1) > 0 && isUnary(ref) && ctx.canAutoTuple =>
2532+
if pt.args.lengthCompare(1) > 0 && takesAutoTupledArgs(ref, ref.symbol, pt.args) =>
25062533
adapt(tree, pt.tupled, locked)
25072534
case _ =>
25082535
adaptOverloaded(ref)

compiler/test/dotty/tools/dotc/CompilationTests.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ class CompilationTests extends ParallelTesting {
180180
compileFilesInDir("tests/neg-custom-args/allow-double-bindings", allowDoubleBindings) +
181181
compileFile("tests/neg-custom-args/i3246.scala", scala2Mode) +
182182
compileFile("tests/neg-custom-args/overrideClass.scala", scala2Mode) +
183-
compileFile("tests/neg-custom-args/autoTuplingTest.scala", defaultOptions.and("-language:noAutoTupling")) +
184183
compileFile("tests/neg-custom-args/i1050.scala", defaultOptions.and("-strict")) +
185184
compileFile("tests/neg-custom-args/nopredef.scala", defaultOptions.and("-Yno-predef")) +
186185
compileFile("tests/neg-custom-args/noimports.scala", defaultOptions.and("-Yno-imports")) +

library/src/scalaShadowing/language.scala

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ package scalaShadowing
2929
* and, for dotty:
3030
*
3131
* - [[Scala2 `Scala2`] backwards compatibility mode for Scala2
32-
* - [[noAtoTupling `noAutoTupling`]] disable auto-tupling
3332
*
3433
* @groupname production Language Features
3534
* @groupname experimental Experimental Language Features
@@ -193,9 +192,6 @@ object language {
193192
/** Where imported, a backwards compatibility mode for Scala2 is enabled */
194193
object Scala2
195194

196-
/** Where imported, auto-tupling is disabled */
197-
object noAutoTupling
198-
199195
/* Where imported loose equality using eqAny is disabled */
200196
object strictEquality
201197
}

tests/neg-custom-args/autoTuplingTest.scala

Lines changed: 0 additions & 9 deletions
This file was deleted.

tests/neg/autoTuplingTest.scala

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
import language.noAutoTupling
2-
31
object autoTuplingNeg2 {
42

53
val x = Some(1, 2) // error: too many arguments for method apply: (x: A)Some[A]
64

75
x match {
8-
case Some(a, b) => a + b // error: wrong number of argument patterns for Some // error: not found: b
6+
case Some(a, b) => a + b // error: wrong number of argument patterns for Some // error: not found: a
97
case None =>
108
}
119
}

tests/neg/i1286.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import scala.io.{ Idontexist4 => Foo } // error
1010
import scala.io.{ Idontexist5 => _ } // error
1111

1212
import scala.language.dynamics
13-
import scala.language.noAutoTupling
1413
import scala.language.idontexist // error
1514

1615
object Test

tests/neg/t6920.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@ class DynTest extends Dynamic {
66

77
class CompilerError {
88
val test = new DynTest
9-
test.crushTheCompiler(a = 1, b = 2) // error
9+
test.crushTheCompiler(a = 1, b = 2) // error // error
1010
}

tests/pos-special/strawman-collections/CollectionStrawMan6.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,7 @@ object CollectionStrawMan6 extends LowPriority {
651651
}
652652

653653
def fromIterator[B](it: Iterator[B]): LazyList[B] =
654-
new LazyList(if (it.hasNext) Some(it.next(), fromIterator(it)) else None)
654+
new LazyList(if (it.hasNext) Some((it.next(), fromIterator(it))) else None)
655655
}
656656

657657
// ------------------ Decorators to add collection ops to existing types -----------------------

tests/pos/autoTuplingTest.scala

Lines changed: 0 additions & 9 deletions
This file was deleted.

tests/pos/i1318.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ object Foo {
33
case class T(i: Int) extends S(i)
44

55
object T {
6-
def unapply(s: S): Option[(Int, Int)] = Some(5, 6)
6+
def unapply(s: S): Option[(Int, Int)] = Some((5, 6))
77
// def unapply(o: Object): Option[(Int, Int, Int)] = Some(5, 6, 7)
88
}
99

@@ -22,7 +22,7 @@ object Bar {
2222
class S(i: Int) extends T(i)
2323

2424
object T {
25-
def unapply(s: S): Option[(Int, Int)] = Some(5, 6)
25+
def unapply(s: S): Option[(Int, Int)] = Some((5, 6))
2626
// def unapply(o: Object): Option[(Int, Int, Int)] = Some(5, 6, 7)
2727
}
2828

tests/pos/i903.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ object Test {
1717
}
1818

1919
def test2 = {
20-
val f = "".contains("", (_: Int)) // dotc:
20+
val f = (x: Int) => "".contains(("", x)) // dotc:
2121
f.apply(0)
2222
// sandbox/eta.scala:18: error: apply is not a member of Boolean(f)
2323
// f.apply(0)

tests/pos/t1133.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,21 @@ object Match
1111

1212
object Extractor1 {
1313
def unapply(x: Any) = x match {
14-
case x: String => Some(x, x + x, x + x+x, x+x, x)
14+
case x: String => Some((x, x + x, x + x+x, x+x, x))
1515
case _ => None
1616
}
1717
}
1818

1919
object Extractor2 {
2020
def unapply(x: Any) = x match {
21-
case x: String => Some(x, x + x, x + x+x)
21+
case x: String => Some((x, x + x, x + x+x))
2222
case _ => None
2323
}
2424
}
2525

2626
object Extractor3 {
2727
def unapply(x: Any) = x match {
28-
case x: String => Some(x, x, x)
28+
case x: String => Some((x, x, x))
2929
case _ => None
3030
}
3131
}

tests/pos/t2913.scala

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ class RichA {
1010
}
1111

1212
object TestNoAutoTupling {
13-
import language.noAutoTupling // try with on and off
14-
1513
implicit def AToRichA(a: A): RichA = new RichA
1614

1715
val a = new A
@@ -52,25 +50,3 @@ object Main {
5250
()
5351
}
5452
}
55-
56-
object TestWithAutoTupling {
57-
58-
implicit def AToRichA(a: A): RichA = new RichA
59-
60-
val a = new A
61-
a.foo()
62-
a.foo(1)
63-
64-
a.foo("") // Without implicits, a type error regarding invalid argument types is generated at `""`. This is
65-
// the same position as an argument, so the 'second try' typing with an Implicit View is tried,
66-
// and AToRichA(a).foo("") is found.
67-
//
68-
// My reading of the spec "7.3 Views" is that `a.foo` denotes a member of `a`, so the view should
69-
// not be triggered.
70-
//
71-
// But perhaps the implementation was changed to solve See https://lampsvn.epfl.ch/trac/scala/ticket/1756
72-
73-
a.foo("a", "b") // Without implicits, a type error regarding invalid arity is generated at `foo(<error>"", "")`.
74-
// Typers#tryTypedApply:3274 only checks if the error is as the same position as `foo`, `"a"`, or `"b"`.
75-
}
76-

0 commit comments

Comments
 (0)