-
Notifications
You must be signed in to change notification settings - Fork 3.1k
SI-4700 Add @infix
annotation for type printing
#5401
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
Conversation
@@ -1405,6 +1405,8 @@ trait Definitions extends api.StandardDefinitions { | |||
case _ => false | |||
} | |||
|
|||
lazy val InfixAnnotationClass = rootMirror.getRequiredClass("scala.annotation.infix") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer getClassIfDefined
to allow the compiler to be able to run with older libraries
|
This wouldn't work for scalactic's
Agreed |
@lrytz @soc You're right, the name is too general. How about I would like to steer clear of:
|
How about Apologies for the bikeshedding ;-) |
Sure, we can call it |
tbh I don't see any problem with the name |
I also don't see a problem with |
``` scala> import scala.annotation.infix import scala.annotation.infix scala> @infix class &&[T, U] defined class $amp$amp scala> def foo: Int && Boolean = ??? foo: Int && Boolean ```
11c9c2f
to
e60768b
Compare
I just pushed a version that updates the name to Also +1 for |
@VladUreche I can see your concern about heuristics, but I think in this case it is simply convention-over-configuration. I think this doesn't deserve yet another tuning knob, of which Scala has too many already. With a simple and clear convention, people who want infix printing know what they need to do. A convention also makes it much easier for readers to see what's happening. |
Are symbolic binary type constructors ever not infix? |
BTW I didn't see anyone mention but I guess this somewhat relates to the proposal for an |
Vlad, you said you were against this, but didn't say why? It seems appealing to me (and others on this thread), but maybe I just need to hear your reasons. |
@SethTisue @lrytz did mention an instance of a non-symbolic infix type constructor... But I don't see the compiler printing |
You should default symbolic names to infix, non-symbolic to non-infix, and have both |
@paulp I'm with Paul on having sane defaults. I disagree on adding annotations. I don't want any annotations to configure more stuff. The cost of adding another two tuning knobs is not worth the benefits of getting the last 0.1%. |
I agree with @paulp on defaults for symbolic names. I imagine this is compatible with most people's intuition. We should still have a mode (compiler option) that prints types in full using a standard format, to make it easier to google them. Once we have an I could be convinced about adding the |
For the record I don't care about the annotations, I only care about the defaults. The question of how to group types is interesting though. trait ~>[F[_], G[_]]
trait ->[+A, +B]
trait F[A]
trait G[A]
scala> def f(x: F ~> G => F ~> G -> Int): Unit = ()
f: (x: ~>[F,G] => ->[~>[F,G],Int])Unit Personally I think the best way to write that type is the way I wrote it: It's a given that if there can only be one way to print a type then there will be huge constituencies who lose. As configurable type printing doesn't appear to be on the horizon, all we can do is lobby for the merits of pleasing certain constituencies. As a member of the "deal with type errors all day long" constituency, anything which lowers the noise is a godsend. |
Thanks for pointing out the precedence issue. I am completely on board with improving type printing this way (print symbolic types infix, minimize parens using precedence) -- the function arrow itself sets the precedent. I'd also love to see a design for an error message/reporting configuration mechanism to improve the (new) user experience. That would be something to flesh out in a doc before prototyping it, though. (I've already done some of the refactorings to make reporting easier to tweak.) |
Sounds good, but right now I'm flooded with work, so it may take a while to update the PR. If anyone has more time on their hands, please go ahead and submit an updated version. |
Closing in favor of #5589, thanks a lot for pushing this forward, @VladUreche! |
Thanks to @lrytz for help!