More robust way to refer to private definitions in types #3167
Conversation
odersky
force-pushed
the
change-private-refs
branch
2 times, most recently
from
3927f4a to
08ad0b6
Compare
|
Previously, we referred to an entity in a NamedType via a prefix type, a name, and (for terms) a signature. This is not enough to distinguish multiple private entities with the same prefix and name or a private definition and a public one with the same prefix and name. We dealt with this using the concept of a "ShadowedName", but it looked more like a patch than a solid fix. We now deal with the problem directly, by adding a nameSpace as a potential third element of a designator. A nameSpace is a TypeRef that indicates where to find a definition. References to private definitions come with a specific nameSpace. |
odersky
changed the title
| this | ||
| else if (signature != denot.signature && denot.signature.ne(Signature.OverloadedSignature)) | ||
| withSig(denot.signature) | ||
| else if (denot.symbol.isPrivate) |
There was a problem hiding this comment.
Indentation is one space too large here, and on the else below.
| @@ -64,8 +64,8 @@ class TastyUnpickler(reader: TastyReader) { | |||
| val result = readName().toTypeName | |||
| val params = until(end)(readName().toTypeName) | |||
| var sig = Signature(params, result) | |||
| if (sig == Signature.NotAMethod) sig = Signature.NotAMethod | |||
| SignedName(original, sig) | |||
| if (sig == Signature.NotAMethod) sig = Signature.NotAMethod // needed temporarily, as long as we read old tasty | |||
There was a problem hiding this comment.
Drop this line and bump the tasty major version instead?
| } | ||
|
|
||
| type TermDesignator = Designator { type ThisName = TermName } | ||
| type TypeDesignator = Designator { type ThisName = TypeName } | ||
|
|
||
| case class LocalName[N <: Name](name: N, nameSpace: TypeRef) extends Designator { |
There was a problem hiding this comment.
Would be nice to document the concept of a LocalName.
There was a problem hiding this comment.
Also, it's a bit confusing that LocalName is not a subclass of Name, I would maybe call it LocalDesignator ?
| @@ -23,8 +26,49 @@ object Designators { | |||
|
|
|||
| def asTerm(implicit ctx: Context): TermDesignator = unsupported("asTerm") | |||
| def asType(implicit ctx: Context): TypeDesignator = unsupported("asType") | |||
|
|
|||
| def withNameSpace(space: NameSpace)(implicit ctx: Context): Designator { type ThisName = self.ThisName } = | |||
There was a problem hiding this comment.
Shouldn't this function be defined in Name instead of Designator?
There was a problem hiding this comment.
No, because it is overridden in LocalName.
The problem is that the assertion in checkNoLeaks might be wrong if the widened type is erroneous. This happened during editing.
Died with an NPE instead of failing the assert
Can be replaced by withNameSpace(noNameSpace).
We admit overloaded signatures in term refs.
Instead, assume that an unsigned name means NotAMethod.
Since these can shadow each other, we need to refer to them by a name and a namespace. This is more solid than the system it replaces, which referred to such symbols symbplically but then fell back to the (unsafe) simple name on pickling.
The had the same signature as polymorphic types, but being a type of types, they should have signature NotAMethod
odersky
force-pushed
the
change-private-refs
branch
from
4fa7a79 to
7f57ff3
Compare
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/3167/ to see the changes. Benchmarks is based on merge(s) with master |
Changes the way we represent references to private data.