SI-8943 Handle non-public case fields in pres. compiler#4081
Merged
retronym merged 1 commit intoscala:2.11.xfrom Nov 2, 2014
Merged
SI-8943 Handle non-public case fields in pres. compiler#4081retronym merged 1 commit intoscala:2.11.xfrom
retronym merged 1 commit intoscala:2.11.xfrom
Conversation
When a case class is type checked, synthetic methods are added,
such as the `hashCode`/`equals`, implementations of the `Product`
interface. At the same time, a case accessor method is added for
each non-public constructor parameter. This the accessor for a
parameter named `x` is named `x$n`, where `n` is a fresh suffix.
This is all done to retain universal pattern-matchability of
case classes, irrespective of access. What is the point of allowing
non-public parameters if pattern matching can subvert access? I
believe it is to enables private setters:
```
case class C(private var x: String)
scala> val x = new C("")
x: C = C()
scala> val c = new C("")
c: C = C()
scala> val C(x) = c
x: String = ""
scala> c.x
<console>:11: error: variable x in class C cannot be accessed in C
c.x
^
scala> c.x = ""
<console>:13: error: variable x in class C cannot be accessed in C
val $ires2 = c.x
^
<console>:10: error: variable x in class C cannot be accessed in C
c.x = ""
^
```
Perhaps I'm missing additional motivations.
If you think scheme sounds like a binary compatiblity nightmare,
you're right: https://issues.scala-lang.org/browse/SI-8944
`caseFieldAccessors` uses the naming convention to find the right
accessor; this in turn is used in pattern match translation.
The accessors are also needed in the synthetic `unapply` method
in the companion object. Here, we must tread lightly to avoid
triggering a typechecking cycles before; the synthesis of that method
is not allowed to force the info of the case class.
Instead, it uses a back channel, `renamedCaseAccessors` to see
which parameters have corresonding accessors.
This is pretty flaky: if the companion object is typechecked
before the case class, it uses the private param accessor directly,
which it happends to have access to, and which duly gets an
expanded name to allow JVM level access. If the companion
appears afterwards, it uses the case accessor method.
In the presentation compiler, it is possible to typecheck a source
file more than once, in which case we can redefine a case class. This
uses the same `Symbol` with a new type completer. Synthetics must
be re-added to its type.
The reported bug occurs when, during the second typecheck, an entry
in `renamedCaseAccessors` directs the unapply method to use `x$1`
before it has been added to the type of the case class symbol.
This commit clears corresponding entries from that map when we
detect that we are redefining a class symbol.
Case accessors are in need of a larger scale refactoring. But I'm
leaving that for SI-8944.
Member
|
LGTM! What a journey.. |
retronym
added a commit
that referenced
this pull request
Nov 2, 2014
SI-8943 Handle non-public case fields in pres. compiler
retronym
added a commit
to retronym/scala
that referenced
this pull request
Nov 10, 2014
Case class parameters that are less that public have an extra accessor method created to ensure universal pattern matchability. See scala#4081 for more background. Currently, this is given a fresh name based on the parameter name. However, this is fragile and the name can change based on unrelated edits higher up in the source file. This commit switches to a stable naming scheme for these methods. A non-public case field `foo` has a corresponding accessor `foo$access$N`, where `N` is the index of the parameter within the constructor parameter list. I have added a backwards compatiblity stub for `::$tl$1` in anticipation of problems with partest otherwise. The enclosed tests show a case that used to trigger a linkage error under separate compilation that now works; shows that by choosing the `foo$access$1` rather than `foo$1` we don't clash with lambda lifted methods in the class; and shows the names of the accessor methods as seen via Java reflection.
retronym
added a commit
to retronym/scala
that referenced
this pull request
Nov 12, 2014
Case class parameters that are less that public have an extra accessor method created to ensure universal pattern matchability. See scala#4081 for more background. Currently, this is given a fresh name based on the parameter name. However, this is fragile and the name can change based on unrelated edits higher up in the source file. This commit switches to a stable naming scheme for these methods. A non-public case field `foo` has a corresponding accessor `foo$access$N`, where `N` is the index of the parameter within the constructor parameter list. The enclosed tests show a case that used to trigger a linkage error under separate compilation that now works; shows that by choosing the `foo$access$1` rather than `foo$1` we don't clash with lambda lifted methods in the class; and shows the names of the accessor methods as seen via Java reflection.
retronym
added a commit
to retronym/scala
that referenced
this pull request
Nov 12, 2014
Case class parameters that are less that public have an extra accessor method created to ensure universal pattern matchability. See scala#4081 for more background. Currently, this is given a fresh name based on the parameter name. However, this is fragile and the name can change based on unrelated edits higher up in the source file. This commit switches to a stable naming scheme for these methods. A non-public case field `foo` has a corresponding accessor `foo$access$N`, where `N` is the index of the parameter within the constructor parameter list. The enclosed tests show a case that used to trigger a linkage error under separate compilation that now works; shows that by choosing the `foo$access$1` rather than `foo$1` we don't clash with lambda lifted methods in the class; and shows the names of the accessor methods as seen via Java reflection.
Contributor
Private setters can be achieved with declaring an explicit def. Given how many hoops we have to go through to support this feature, could we consider deprecating it? |
Member
Author
|
How would we rewrite this? final case class ::[B](override val head: B, private[scala] var tl: List[B]) extends List[B] {
override def tail : List[B] = tl
override def isEmpty: Boolean = false
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a case class is type checked, synthetic methods are added,
such as the
hashCode/equals, implementations of theProductinterface. At the same time, a case accessor method is added for
each non-public constructor parameter. This the accessor for a
parameter named
xis namedx$n, wherenis a fresh suffix.This is all done to retain universal pattern-matchability of
case classes, irrespective of access. What is the point of allowing
non-public parameters if pattern matching can subvert access? I
believe it is to enables private setters:
Perhaps I'm missing additional motivations.
If you think scheme sounds like a binary compatiblity nightmare,
you're right: https://issues.scala-lang.org/browse/SI-8944
caseFieldAccessorsuses the naming convention to find the rightaccessor; this in turn is used in pattern match translation.
The accessors are also needed in the synthetic
unapplymethodin the companion object. Here, we must tread lightly to avoid
triggering a typechecking cycles before; the synthesis of that method
is not allowed to force the info of the case class.
Instead, it uses a back channel,
renamedCaseAccessorsto seewhich parameters have corresonding accessors.
This is pretty flaky: if the companion object is typechecked
before the case class, it uses the private param accessor directly,
which it happends to have access to, and which duly gets an
expanded name to allow JVM level access. If the companion
appears afterwards, it uses the case accessor method.
In the presentation compiler, it is possible to typecheck a source
file more than once, in which case we can redefine a case class. This
uses the same
Symbolwith a new type completer. Synthetics mustbe re-added to its type.
The reported bug occurs when, during the second typecheck, an entry
in
renamedCaseAccessorsdirects the unapply method to usex$1before it has been added to the type of the case class symbol.
This commit clears corresponding entries from that map when we
detect that we are redefining a class symbol.
Case accessors are in need of a larger scale refactoring. But I'm
leaving that for SI-8944.
Review by @dragos @lrytz