Skip to content
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

Fix #9028: Introduce super traits #9032

Merged
merged 9 commits into from
Jun 10, 2020

Conversation

odersky
Copy link
Contributor

@odersky odersky commented May 24, 2020

No description provided.

@@ -1304,6 +1303,9 @@ class Definitions {
def isInfix(sym: Symbol)(implicit ctx: Context): Boolean =
(sym eq Object_eq) || (sym eq Object_ne)

@tu lazy val assumedSuperTraits =
Set(ComparableClass, JavaSerializableClass, ProductClass, SerializableClass)
Copy link
Member

Choose a reason for hiding this comment

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

SerializableClass is identical to JavaSerializableClass (it used to be different, but since 2.13 scala.Serializable is an alias of java.io.Serializable)

@smarter
Copy link
Member

smarter commented May 24, 2020

I like the idea overall, some remarks:

  • I'm not sure about the keyword. The term "supertrait" is already commonly used to refer to a parent trait in the same way that the term superclass is used. In fact, it's so common that it's in the official scala glossary!
  • I could see this mechanism also being useful for classes and not just traits, for example the standard library contains classes such as AbstractSeq which only exist to avoid generating mixin forwarders in every leaf class.
  • This could also be useful to avoid inferring a base class which is not accessible in the current scope (in fact, the Abstract* classes in the standard library used to be private but were made public because they leaked into inferred types)

@odersky
Copy link
Contributor Author

odersky commented May 25, 2020

I can see some use cases for dropping classes, but the existing rules do not lend themselves to be generalized that way. We'd have to come up with a different concept what would get dropped when.

That said, I believe the issue with traits is a lot more severe than the issue with classes. So I am not sure we need to go further than just traits.

@smarter
Copy link
Member

smarter commented May 25, 2020

We'd have to come up with a different concept what would get dropped when.

Right, so the conservative thing to do would be to not get this in 3.0 in case we come up with some different rules, but maybe keep the special case for Serializable/Product/... since those are the most common offenders?

@odersky
Copy link
Contributor Author

odersky commented May 25, 2020

If we change Serializable and Product we already have to buy into the whole scheme. The hairy part is to develop rules what to drop when, and this is already exercised by those two.

It feels a bit ad hoc to me to just keep it to those two classes. Whereas restricting it to traits is clearly a step in the right direction (and maybe the only one we have to take).

@odersky odersky marked this pull request as ready for review May 25, 2020 17:07
@SethTisue
Copy link
Member

SethTisue commented May 28, 2020

Would/could this help typing of collections? To e.g. improve the inferred type of Seq(List(0), Vector(0))?

Starting dotty REPL...
scala> Seq(List(0), Vector(0))                                                                                          
val res0: 
  Seq[scala.collection.immutable.AbstractSeq[Int] & 
    scala.collection.immutable.StrictOptimizedSeqOps[Int, 
      [X0] =>> List[X0] | Vector[X0]
    , List[Int] | Vector[Int]]
   & scala.collection.generic.DefaultSerializable] = List(List(0), Vector(0))

@Odomontois
Copy link

Odomontois commented May 28, 2020

Isn't the problem of too narrow inference already solved for most cases with the enum addition?
What are some known issues fixed by this syntax that could not\should not be fixed by enums?

@odersky
Copy link
Contributor Author

odersky commented May 29, 2020

@SethTisue With the latest changes:

scala> Seq(List(0), Vector(0))
val res0: Seq[scala.collection.immutable.AbstractSeq[Int]] = List(List(0), Vector(0))

@Odomontois This also shows that the problem is more general than what can be solved with enums.

@Jasper-M
Copy link
Contributor

Jasper-M commented Jun 3, 2020

I'm not sure about the keyword. The term "supertrait" is already commonly used to refer to a parent trait in the same way that the term superclass is used. In fact, it's so common that it's in the official scala glossary!

I also don't really see the relation between the keyword super and what a super trait does. Actually before I read the rules in the PR I assumed that the super trait would be the one that does get inferred.

Maybe impl trait? Since it's about traits that only provide implementations but are not interesting on their own.

@odersky
Copy link
Contributor Author

odersky commented Jun 8, 2020

I also don't really see the relation between the keyword super and what a super trait does. Actually before I read the rules in the PR I assumed that the super trait would be the one that does get inferred.

A super trait is a trait that is intended to be used specifically as a super trait of some other traits or classes, not as a type by itself. I first found it a bit weird, but now find it quite natural and better than having to invent another keyword. impl in particular does not cut it, since it does not decscribe F-bounded interfaces such as Comparable.

@sjrd
Copy link
Member

sjrd commented Jun 8, 2020

I'm not very convinced by this proposal.

I understand the problem it solves, and I agree that it would be nice if this problem was solved. However, I don't think this proposal is an appropriate solution.

My main concern is that this seems to be a language feature whose only purpose is to alter type inference. I do not think there is any precedent for that in Scala. It is concerning to me because type inference in general is not specified at all. So now we have one specific language feature, which needs a specification, whose effect is to alter a process that is completely unspecified. How does one do that? I do not think it can be done.

@SethTisue
Copy link
Member

this seems to be a language feature whose only purpose is to alter type inference

Perhaps it's a secondary concern, but I think it could also lead to better Scaladoc, since generated Scaladoc could present the "main" traits first and more prominently, and the super traits secondarily. This would certainly make the Scaladoc for the standard collections more digestible.

@SethTisue
Copy link
Member

SethTisue commented Jun 8, 2020

Even after a couple weeks of rolling the name "super trait" around in my brain and trying to get used to it, I remain highly uncomfortable with it.

"Superclass" is one of the most established words in all of object-oriented programming. To introduce "super trait" but giving it a significant difference in meaning is a bad move, I fear.

Some Googling shows that in a Scala context, both of the forms "supertrait" and "super trait" are commonly used, simply to mean any trait a type inherits. That's exactly what one would expect it to mean, and exactly how people are already using it, given how established the term "superclass" is.

@odersky "supertrait" is even already used that way in your own book.

@odersky
Copy link
Contributor Author

odersky commented Jun 8, 2020

@odersky "supertrait" is even already used that way in your own book.

Yes, and it is used in exactly that sense here! Every trait can be a super trait. By adding super to the trait definition, we indicate that the trait is commonly used in that role and no other. I am open to other proposals, but have not seen anything to consider yet.

EDIT: To use an analogy with human parents: Lisa is the mother of Paul, but one can also say Lisa is a mother. One can even say mother Lisa (even though then she's most likely not a mother of someone, but that's another matter).

@sideeffffect
Copy link
Contributor

The topic of super traits is very interesting to me even as a mere Scala user.
The related ticket #9028 introduces the concept with

Ross Tate et al make an interesting observation in their paper "Getting F-bounded polymorphism into shape": Traits split often cleanly into "materials" and "shapes". Materials in their terminology are traits that are (parts of) types of vals and defs whereas shapes are only used as super-traits of other classes and traits. They point out that traits inherited recursively that give rise to F-bounded polymorphism are in practice always shapes, and suggest that algorithms for subtyping and type checking could be simplified if this rule was enforced.

Is the separation between "materials" and "shapes" now enforced? Has it (can it) lead to simplification of algorithms for subtyping and/or type checking in Scala (i.e. Dotty)?

@odersky
Copy link
Contributor Author

odersky commented Jun 8, 2020

My main concern is that this seems to be a language feature whose only purpose is to alter type inference. I do not think there is any precedent for that in Scala.

Singleton is a precedent. In any case, good type inference is the primary concern when you use Scala. It's both the biggest convenience when it works and the subject of horror stories when it does not. The fact that it is not specified in prose or formulas is a pity, but that should not be a reason not to do anything about it.

@odersky
Copy link
Contributor Author

odersky commented Jun 9, 2020

Perhaps it's a secondary concern, but I think it could also lead to better Scaladoc, since generated Scaladoc could present the "main" traits first and more prominently, and the super traits secondarily. This would certainly make the Scaladoc for the standard collections more digestible.

Good point. We should do this, and I agree that it would help immensely.

odersky added 9 commits June 9, 2020 11:05
and eliminate super traits in widenInferred
Drop supertraits only if they are a mixin to something else (which is not
a supertype of the supertrait itself).
This can be added in a backwards compatible way to cross-compiling sources
The idea is that some traits the 2.13 standard library should get annotated
with @superTrait. I added the annotations to our version in the community build.
Until that is done, we assume that the annotated traits are super traits by
adding them to the assumedSuperTraits set.
@odersky
Copy link
Contributor Author

odersky commented Jun 9, 2020

I'd like to get this into 0.25. I think the benefits of this change are very promising and we should gain experience sooner rather than later.

@odersky odersky added this to the 0.25.0-RC1 milestone Jun 9, 2020
@anatoliykmetyuk anatoliykmetyuk merged commit 7536df9 into scala:master Jun 10, 2020
@anatoliykmetyuk anatoliykmetyuk deleted the fix-#9028 branch June 10, 2020 10:09
@amitport
Copy link

amitport commented Jun 12, 2020

May I suggest using mixin or mixin trait instead of super trait. It seems a lot more self-explanatory to me.

Additionally, note that the current documentation of mixin-class-composition starts with "Mixins are traits which are used to compose a class", so this seems in line with scala's terminology.

@sjrd
Copy link
Member

sjrd commented Jun 12, 2020

I can't come to terms with the name "super trait". As already explained by @SethTisue and @smarter, "super trait" is already used for a more general concept than what has been implemented here. Yes, every trait can be a super trait; that's not the issue. The issue is that every trait is already a super trait, for the already widely used meaning of super trait. And even if "super trait" should mean this new meaning here, there is no way that "super class" is going to change from its decades-old meaning, so now users are even more confused because a "super trait" is not to be inferred, but a "super class" is inferred. How do you teach the difference between a super class and a super trait?

I don't even understand how "super trait" can even be considered. The only reason I saw was "because the keyword already exist" with everything else sounding more like rationalization than intended design. There was this:

A super trait is a trait that is intended to be used specifically as a super trait of some other traits or classes, not as a type by itself.

How do you reconcile that with the fact that the entire PL world talks about super types? A super trait is specifically intended not to be used as a super type. Seriously?

If the reason is "because the keyword already exist", I'd throw in "extends trait". It's a trait meant to be used in an extends clause and not elsewhere. Yes, it's horrible, but it's not misleading. I am not actually recommending this; I am pointing out how poor a reason it is to name a concept simply by looking at what keywords do we have and how can we "reuse" them.

"Mixin trait" is not bad. At least we don't have years of literature talking about "mixin traits" for something different than what this PR implements. I'll also throw in "no-infer trait" as an ugly-but-obvious name.

@jducoeur
Copy link
Contributor

Chiming in from the end-user peanut gallery: strong agreement with @sjrd -- this terminology is used far too widely, and far too casually, for this to be any sort of good idea. It's guaranteed to cause confusion; worse, it's guaranteed to cause miscommunication, and speaking as someone who use Scala every day in a corporate environment, that's always my worst nightmare.

Choose some jargon instead -- "mixin trait" would be fine, just something that isn't already laden with incorrect connotation...

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.

10 participants