From 019dccbc38bfdb9c8d11348e5f6be317142ca810 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 19 Nov 2018 18:34:12 +0100 Subject: [PATCH 1/4] Restrict visibility of copy and apply Make the synthesized copy and apply methods of a case class have the same visibility as its private constructor. --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 4 ++-- tests/neg/private-case-class-constr.scala | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 tests/neg/private-case-class-constr.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index ab3bb6897ed2..7a66f415a71a 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -469,7 +469,7 @@ object desugar { val copyRestParamss = derivedVparamss.tail.nestedMap(vparam => cpy.ValDef(vparam)(rhs = EmptyTree)) DefDef(nme.copy, derivedTparams, copyFirstParams :: copyRestParamss, TypeTree(), creatorExpr) - .withMods(synthetic) :: Nil + .withFlags(Synthetic | constr1.mods.flags & AccessFlags) :: Nil } } @@ -574,7 +574,7 @@ object desugar { if (mods is Abstract) Nil else DefDef(nme.apply, derivedTparams, derivedVparamss, applyResultTpt, widenedCreatorExpr) - .withFlags(Synthetic | (constr1.mods.flags & DefaultParameterized)) :: widenDefs + .withFlags(Synthetic | constr1.mods.flags & (DefaultParameterized | AccessFlags)) :: widenDefs val unapplyMeth = { val unapplyParam = makeSyntheticParameter(tpt = classTypeRef) val unapplyRHS = if (arity == 0) Literal(Constant(true)) else Ident(unapplyParam.name) diff --git a/tests/neg/private-case-class-constr.scala b/tests/neg/private-case-class-constr.scala new file mode 100644 index 000000000000..b8a045e62e89 --- /dev/null +++ b/tests/neg/private-case-class-constr.scala @@ -0,0 +1,10 @@ +case class Nat private (value: Int) +object Nat { + def apply(value: Int, disable: Boolean): Option[Nat] = + if (value < 0 || disable) None else Some(new Nat(value)) +} +object Test { + val n1o = Nat(2) // error + val n2o = Nat(2, false) // ok + for (n <- n2o) yield n.copy() // error +} \ No newline at end of file From cb5c76e5bd653c1c1c5c1df2beffc593561e0f18 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 19 Nov 2018 21:01:43 +0100 Subject: [PATCH 2/4] Fix tests and bootstrapped code There were two occurrences where we exploited that a case class with a private constructor still had a public apply method. I believe both were mistakes. --- compiler/src/dotty/tools/dotc/interactive/Interactive.scala | 2 +- tests/pos/i4564.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index b33d6c0407e1..77b0b5ec134c 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -22,7 +22,7 @@ object Interactive { import ast.tpd._ object Include { - case class Set private (val bits: Int) extends AnyVal { + case class Set private[Include] (val bits: Int) extends AnyVal { def | (that: Set): Set = Set(bits | that.bits) def except(that: Set): Set = Set(bits & ~that.bits) diff --git a/tests/pos/i4564.scala b/tests/pos/i4564.scala index fce09873a40f..3334d219e805 100644 --- a/tests/pos/i4564.scala +++ b/tests/pos/i4564.scala @@ -51,7 +51,7 @@ class BaseNCP[T] { } object NoClashPoly extends BaseNCP[Boolean] -case class NoClashPoly private(x: Int) +case class NoClashPoly (x: Int) class BaseCP[T] { From d657bd793dc1163915971a30dc2e46026ead9e1d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 21 Nov 2018 20:54:14 +0100 Subject: [PATCH 3/4] Drop unused definition --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 7a66f415a71a..fe4f72eb0a19 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -658,8 +658,6 @@ object desugar { flatTree(cdef1 :: companions ::: implicitWrappers) } - val AccessOrSynthetic: FlagSet = AccessFlags | Synthetic - /** Expand * * object name extends parents { self => body } From 6b840fc62d1b162fbc1c1fb0f3b271b110c3bd06 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 21 Nov 2018 21:09:02 +0100 Subject: [PATCH 4/4] Keep old scheme if option -language:Scala2 is set. --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 6 ++++-- compiler/src/dotty/tools/dotc/core/TypeOps.scala | 5 +++++ compiler/src/dotty/tools/dotc/parsing/Scanners.scala | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index fe4f72eb0a19..f634ba8e28e0 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -430,6 +430,8 @@ object desugar { // new C[Ts](paramss) lazy val creatorExpr = New(classTypeRef, constrVparamss nestedMap refOfDef) + val copiedAccessFlags = if (ctx.scala2Setting) EmptyFlags else AccessFlags + // Methods to add to a case class C[..](p1: T1, ..., pN: Tn)(moreParams) // def _1: T1 = this.p1 // ... @@ -469,7 +471,7 @@ object desugar { val copyRestParamss = derivedVparamss.tail.nestedMap(vparam => cpy.ValDef(vparam)(rhs = EmptyTree)) DefDef(nme.copy, derivedTparams, copyFirstParams :: copyRestParamss, TypeTree(), creatorExpr) - .withFlags(Synthetic | constr1.mods.flags & AccessFlags) :: Nil + .withFlags(Synthetic | constr1.mods.flags & copiedAccessFlags) :: Nil } } @@ -574,7 +576,7 @@ object desugar { if (mods is Abstract) Nil else DefDef(nme.apply, derivedTparams, derivedVparamss, applyResultTpt, widenedCreatorExpr) - .withFlags(Synthetic | constr1.mods.flags & (DefaultParameterized | AccessFlags)) :: widenDefs + .withFlags(Synthetic | constr1.mods.flags & (DefaultParameterized | copiedAccessFlags)) :: widenDefs val unapplyMeth = { val unapplyParam = makeSyntheticParameter(tpt = classTypeRef) val unapplyRHS = if (arity == 0) Literal(Constant(true)) else Ident(unapplyParam.name) diff --git a/compiler/src/dotty/tools/dotc/core/TypeOps.scala b/compiler/src/dotty/tools/dotc/core/TypeOps.scala index 857b8e59ef59..aa6ce6207706 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeOps.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeOps.scala @@ -327,6 +327,11 @@ trait TypeOps { this: Context => // TODO: Make standalone object. } scala2Mode } + + /** Is option -language:Scala2 set? + * This test is used when we are too early in the pipeline to consider imports. + */ + def scala2Setting = ctx.settings.language.value.contains(nme.Scala2.toString) } object TypeOps { diff --git a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala index 63f907af6b5c..4fce70cfdcf9 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala @@ -242,7 +242,7 @@ object Scanners { // Scala 2 compatibility - val isScala2Mode: Boolean = ctx.settings.language.value.contains(nme.Scala2.toString) + val isScala2Mode: Boolean = ctx.scala2Setting /** Cannot use ctx.featureEnabled because accessing the context would force too much */ def testScala2Mode(msg: String, pos: Position = Position(offset)): Boolean = {