Skip to content

Several type system fixes #4137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 19, 2018
Merged

Several type system fixes #4137

merged 8 commits into from
Mar 19, 2018

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 18, 2018

Prompted by a study of possible typeclass encodings several small fixes to aspects of type checking.

@@ -1967,6 +1967,7 @@ object Types {
def derivedSelect(prefix: Type)(implicit ctx: Context): Type =
if (prefix eq this.prefix) this
else if (prefix.isBottomType) prefix
else if (prefix.isInstanceOf[WildcardType]) WildcardType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's what look like an identical case at the end of this method (case _: WildcardType => WildcardType), can the two be merged?

odersky added 5 commits March 18, 2018 22:12
Allow several type aliases if they are the same.
One involved selections on WildcardTypes,
the other taking the base type of refinement classes.

Both manifested themselves in neg/typeclass-encoding2.scala
Sometimes a computation of a superType will give NoType. Keeping with the spirit of unKnownMembers to say yes when in doubt, we should return true in this case.

The change is necessary to make the recursive call of `develop` and the call to `flatMap` in `flatten` in typeclass-encoding2.scala typecheck.
type This = Int
type Instance = Monoid
def unit: Int = 0
def inject($this: Int) = new Monoid {
val common: self.type = self
val common: IntOps.this.type = IntOps.this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be : IntOps.type = IntOps

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the same, but less regular. I am after an encoding that works the same for classes and objects. Unfortunately, with self types for objects dropped, we only have qualified this to avoid an irregularity.

odersky added 2 commits March 18, 2018 22:27
Needed to make latest instance of typeclass-encoding2 pass. Compared to previous
this one has a single type parameter for `Extension`.
@odersky
Copy link
Contributor Author

odersky commented Mar 19, 2018

test performance please

@dottybot
Copy link
Member

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

@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (990469e)

 - Add straight extensions of type classes, i.e. implementation inherits type class directly.
 - Rename common as an identifier to commons
 - Drop `Extension` type.
@odersky
Copy link
Contributor Author

odersky commented Mar 19, 2018

test performance please

@dottybot
Copy link
Member

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

@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (990469e)

@odersky odersky merged commit 9c6060a into scala:master Mar 19, 2018
@Blaisorblade Blaisorblade deleted the add-thistypes branch March 19, 2018 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants