Generalize RefTypes #3087

odersky · 2017-09-08T15:06:14Z

Based on #3061.

The aim of this PR is to eliminate the various subtypes of TermRef and TypeRef and replace them by generalizing Name Name/Signature and Symbol to a new concept of Designator.

The expected benefits of doing this are:

Drop the kludgey way we deal with "shadowed names" by making all private references symbolic
Allow to eliminate TypeArgRefs

dottybot

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

Separate subject from body with a blank line

When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:

Limit the subject line to 72 characters

Capitalize the subject line

Do not end the subject line with a period

Use the imperative mood in the subject line ("Added" instead of "Add")

Wrap the body at 80 characters

Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

odersky · 2017-09-13T11:53:15Z

test performance please

dottybot · 2017-09-13T11:53:30Z

performance test scheduled: 1 job(s) in queue.

dottybot · 2017-09-13T12:29:33Z

performance test failed:

[check /data/workspace/bench/logs/pull-3087-09-13-14.05.out for more information]

odersky · 2017-09-13T14:34:53Z

test performance please

dottybot · 2017-09-13T14:35:32Z

performance test scheduled: 1 job(s) in queue, 1 running.

liufengyun · 2017-09-13T14:40:13Z

@odersky I just checked the logs, compiling of dotty project succeeds, but compilation of the scala-library project throws exception.

smarter · 2017-09-13T15:02:00Z

@lihaoyi Oh, the scala-library in the benchmarks is not the same as the one from the submodule? I think that would make more sense.

dottybot · 2017-09-13T15:19:01Z

performance test failed:

[check /data/workspace/bench/logs/pull-3087-09-13-16.55.out for more information]

liufengyun · 2017-09-13T15:38:54Z

@smarter Exactly, it's not the same scala-library in the Dotty repo. It's intentionally made to be not the same, as the Dotty scala-library is growing, and we want a subset of stdlib that could be compiled since last october.

odersky · 2017-09-14T08:16:23Z

test performance please

dottybot · 2017-09-14T08:16:49Z

performance test scheduled: 1 job(s) in queue, 0 running.

dottybot · 2017-09-14T08:56:33Z

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3087 to see the changes.

Benchmarks is based on merge(s) with master

AppliedType will be used for all type applications, higher-kinded or not. That is, eventually, it will replace HKApply and also the encodings of type applications as refined types. This commit introduces AppliedType and adapts baseType computations to take it into account. AppliedType is not yet constructed anywhere, however.

So far, everything up to parents is adapted to new scheme. Parents is half done; needs more work once we change layour of ClassInfo.

Simplifies usage and removes some obscure code in Types

Systematically introduce handlers for AppliedTypes where there was a handler for HKApply.

Also, change asSeenFrom to use it for arguments

withfixedSym -> apply It's just apply with a different designator apply(Type, Symbol) -> withSym This one contains special cases.

Make nonMemberTermRef redundant by marking local dummies and import symbols as Fresh (i.e. they are non-members).

Also - rename Fresh flag to NonMember - update criterion for phases where refs are symbolic. This used to be just before erasure (at resolveSuper) but then other phases were inserted after resolveSuper and the test became outdated. It's more robust now.

- drop withSym - rename withSymAndName to another apply - streamline select methods

To do this, we needed a more robust version of Symbol#signature.

Also, drop signature parameter in TermRef.withSigAndDenot

The exception previously made for Scala2 innner classes causes the benchmark library project to fail with in the tree type map FullParameterization when called from LinkScala2Impl. It's not necessary to make the special case there because the Scalq2Unpickler will create anyway symbolic references to such inner classes.

smarter · 2017-09-20T15:30:09Z

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

+import Names._
+
+/** Defines a common superclass of Name and Symbol and its Term/Type variants
+ *  Designators are used in reference type to identity what is referred to.


typo: identity -> identify

smarter · 2017-09-20T15:30:32Z

docs/docs/reference/multiversal-equality.md

+    Here _lifting_ a type `S` means replacing all references to  abstract types
+    in covariant positions of `S` by their upper bound, and to replacing
+    all refinement types in in covariant positions of `S` by their parent.
+    The purpose if lifte


Sentence wasn't finished.

It allows us to drop the additional name field in most cases and the methods become easier to discover.

smarter · 2017-09-20T16:23:13Z

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

@@ -1921,7 +1919,10 @@ object Types {
     *  the public name.
     */
    def shadowed(implicit ctx: Context): NamedType =
-      NamedType(prefix, designatorName.derived(ShadowedName))
+      designator match {
+        case designator: Symbol => this


Is this case ever reached? It doesn't really seem to make sense.

smarter · 2017-09-20T16:27:28Z

compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala

+      }
+      else if (tpe.hasFixedSym) {
+        assert(tpe.symbol.isClass && tpe.symbol.is(Flags.Scala2x), tpe.symbol.showLocated)
+        writeByte(TYPEREF) // should be changed to a new entry that keeps track of prefix, symbol & owner


Is this still a TODO?

odersky · 2017-09-21T10:04:14Z

Withdrawn in favor of #3147

dottybot reviewed Sep 8, 2017

View reviewed changes

odersky force-pushed the change-termref branch 8 times, most recently from 48231de to 0b4b689 Compare September 13, 2017 09:22

odersky added 9 commits September 20, 2017 09:46

Adapt operations in Types

7be50a6

So far, everything up to parents is adapted to new scheme. Parents is half done; needs more work once we change layour of ClassInfo.

Move givenSelfType to ClassSymbol

2ec644b

Simplifies usage and removes some obscure code in Types

Handle AppliedTypes

b219c74

Systematically introduce handlers for AppliedTypes where there was a handler for HKApply.

Introduce TypeArgRef

6a63549

Also, change asSeenFrom to use it for arguments

Various fixes

4b64c00

More fixes

76d1194

Fix stray brace

cf491ff

Fix typing of _* arguments

08ff2b2

odersky added 16 commits September 20, 2017 17:26

Get rid of FixedSymUniques

6bca2da

Get rid of FixedSymUniques

a3c69a5

Get rid of WithFixedSym

d868f33

Renamings of creation methods

2539e47

withfixedSym -> apply It's just apply with a different designator apply(Type, Symbol) -> withSym This one contains special cases.

Drop nonMemberTermRef

b82b2fa

Make nonMemberTermRef redundant by marking local dummies and import symbols as Fresh (i.e. they are non-members).

Drop Fresh in mapSymbols

df0057c

Streamline NamedType creation methods

483a8f3

- drop withSym - rename withSymAndName to another apply - streamline select methods

Eliminate TermRef.withSig

760479b

To do this, we needed a more robust version of Symbol#signature.

Eliminate termRefWithSig and valRef

98d8ecf

Also, drop signature parameter in TermRef.withSigAndDenot

Eliminate termRefWithSig and valRef

f941b4f

Also, drop signature parameter in TermRef.withSigAndDenot

Cleanup pickling of NamedTypes

66d61d4

Eliminate designatorName

5cd5ff0

Tweak diagndostic message

24c2909

Rename Designator(s).scala

b08b1a9

smarter reviewed Sep 20, 2017

View reviewed changes

odersky force-pushed the change-termref branch from a458730 to 4241475 Compare September 20, 2017 15:30

Go back to withSym creation methods for NamedTypes

2f9d66c

It allows us to drop the additional name field in most cases and the methods become easier to discover.

smarter reviewed Sep 20, 2017

View reviewed changes

smarter approved these changes Sep 20, 2017

View reviewed changes

odersky closed this Sep 21, 2017

odersky mentioned this pull request Sep 22, 2017

WIP: Change namedtypes #3147

Closed

odersky reopened this Sep 22, 2017

odersky changed the title ~~[WIP] Generalize RefTypes~~ Generalize RefTypes Sep 22, 2017

odersky merged commit 78e80c8 into scala:master Sep 22, 2017

allanrenucci deleted the change-termref branch December 14, 2017 16:58

Generalize RefTypes #3087

Generalize RefTypes #3087

Uh oh!

Conversation

odersky commented Sep 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dottybot left a comment

Choose a reason for hiding this comment

Commit Messages

Uh oh!

odersky commented Sep 13, 2017

Uh oh!

dottybot commented Sep 13, 2017

Uh oh!

dottybot commented Sep 13, 2017

Uh oh!

odersky commented Sep 13, 2017

Uh oh!

dottybot commented Sep 13, 2017

Uh oh!

liufengyun commented Sep 13, 2017

Uh oh!

smarter commented Sep 13, 2017

Uh oh!

dottybot commented Sep 13, 2017

Uh oh!

liufengyun commented Sep 13, 2017

Uh oh!

odersky commented Sep 14, 2017

Uh oh!

dottybot commented Sep 14, 2017

Uh oh!

dottybot commented Sep 14, 2017

Uh oh!

smarter Sep 20, 2017

Choose a reason for hiding this comment

Uh oh!

smarter Sep 20, 2017

Choose a reason for hiding this comment

Uh oh!

smarter Sep 20, 2017

Choose a reason for hiding this comment

Uh oh!

smarter Sep 20, 2017

Choose a reason for hiding this comment

Uh oh!

odersky commented Sep 21, 2017

Uh oh!

Uh oh!

odersky commented Sep 8, 2017 •

edited

Loading