-
Notifications
You must be signed in to change notification settings - Fork 19
Add an @implicitAmbiguous annotation #32
Add an @implicitAmbiguous annotation #32
Conversation
This is a fix for SI-6806. My commit raises a question. I've added a class to scala-library.jar - can/should we require users of Typelevel Scala to use a (binary compatible) version of scala-library? Should we instead make them use the standard scala-library.jar and just provide an additional library instead? |
Found two failing tests:
|
@@ -1091,6 +1091,7 @@ trait Definitions extends api.StandardDefinitions { | |||
lazy val BridgeClass = requiredClass[scala.annotation.bridge] | |||
lazy val ElidableMethodClass = requiredClass[scala.annotation.elidable] | |||
lazy val ImplicitNotFoundClass = requiredClass[scala.annotation.implicitNotFound] | |||
lazy val ImplicitAmbiguousClass = requiredClass[scala.annotation.implicitAmbiguous] |
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.
You probably want to make this getClassIfDefined
and handle the missing annotation gracefully (see CompileTimeOnlyAttr
, for example).
If you only need additive library changes, consider an extra JAR like scala-library-tlc.jar
. You could save (or at least defer) a lot of cost by sticking to compiler only changes in the short term.
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.
@retronym awesome, thank you!
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.
If going the extra JAR route, I strongly advocate the use of a new package to avoid the known and hated split package issues, e.g. when working in OSGi or with sealed packages.
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.
@lefou absolutely with you.
Seems that to make scala-library-typelevel.jar possible also requires us to fork partest as well as update the build.xml awfulness. What's the possibility of using @paulp's new sbt-based build? |
@puffnfresh I can imagine a world where we collaborate on the build infrastructure. This in fact is the main reason I released it now, so as not to miss that opportunity. The code really is not in the state I would have chosen though. What I'm presently in the progress of is extracting most of that code and combining it with a bunch of other sbt code into "libsbt", my attempt at making sbt somewhat usable. You can see the dependency in policy project/plugins.sbt but I haven't published the source yet (except inasmuch as I assume there are source jars alongside the plugin.) Once I complete that process the amount of building-scala-specific code in the policy project will be relatively small. |
@paulp so we would just add libsbt as an sbt dep and be pretty much good to go? |
@puffnfresh Geez it isn't going to be anywhere near that easy. |
Can one of the admins verify this patch? |
8b730e6
to
4d4f703
Compare
Example usage: trait =!=[C, D] implicit def neq[E, F] : E =!= F = null @annotation.implicitAmbiguous("Could not prove ${J} =!= ${J}") implicit def neqAmbig1[G, H, J] : J =!= J = null implicit def neqAmbig2[I] : I =!= I = null implicitly[Int =!= Int] Which gives the following error: implicit-ambiguous.scala:9: error: Could not prove Int =!= Int implicitly[Int =!= Int] ^ Better than what was previously given: implicit-ambiguous.scala:9: error: ambiguous implicit values: both method neqAmbig1 in object Test of type [G, H, J]=> Main.$anon.Test.=!=[J,J] and method neqAmbig2 in object Test of type [I]=> Main.$anon.Test.=!=[I,I] match expected type Main.$anon.Test.=!=[Int,Int] implicitly[Int =!= Int] ^
add to whitelist |
I know this potentially makes thing much more complex, but I have to say I'm slightly annoyed by the fact we have to provide a dummy body ;-) |
@aloiscochard what's much more complex? |
Well actually I don't know, just a wild guess than making it work the way I I hope I'm just being pessimistic for no good reason?
|
@aloiscochard I'm not sure what you proposed. I think I now understand what you mean by "dummy body" though. If you don't like the fact that it's not total, you could actually provide the same implementation as |
As discussed on IRC with @puffnfresh, an other more ad-hoc alternative to achieve the "real" goal (disabling implicits) would be indeed much more complex and take longer. Thanks! 👍 |
This is where our additions to scala-library.jar will go. The current content is the implicitAmbiguous annotation. I seemed to have made the right changes to make the REPL, the test classpath and scala-dist to work. I'm not sure if there's anything else we need but I also inserted references to typelevel where library was also referenced, just in case.
@milessabin @non @propensive a review on my last commit here would be useful. Is this the direction we want to go? |
I think that's the right thing. We're going to need that jar one way or another. |
Yes, I agree. |
…otation Add an @implicitAmbiguous annotation
Non local returns aren't eliminated after inlined in 2.11 or 2.12 ``` ⚡ scala Welcome to Scala 2.12.1 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_112). Type in expressions for evaluation. Or try :help. scala> @inlune def foo(a: => Any) = if ("".isEmpty) a else "" <console>:11: error: not found: type inlune @inlune def foo(a: => Any) = if ("".isEmpty) a else "" ^ scala> @inline def foo(a: => Any) = if ("".isEmpty) a else "" foo: (a: => Any)Any scala> class InlineReturn { def test: Any = foo(return "") } defined class InlineReturn scala> :javap -c InlineReturn#test public java.lang.Object test(); Code: 0: new #4 // class java/lang/Object 3: dup 4: invokespecial #32 // Method java/lang/Object."<init>":()V 7: astore_1 8: getstatic #36 // Field $line4/$read$$iw$$iw$.MODULE$:L$line4/$read$$iw$$iw$; 11: aload_1 12: invokedynamic #59, 0 // InvokeDynamic #0:apply:(Ljava/lang/Object;)Lscala/Function0; 17: invokevirtual #63 // Method $line4/$read$$iw$$iw$.foo:(Lscala/Function0;)Ljava/lang/Object; 20: goto 44 23: astore_2 24: aload_2 25: invokevirtual #66 // Method scala/runtime/NonLocalReturnControl.key:()Ljava/lang/Object; 28: aload_1 29: if_acmpne 39 32: aload_2 33: invokevirtual #69 // Method scala/runtime/NonLocalReturnControl.value:()Ljava/lang/Object; 36: goto 41 39: aload_2 40: athrow 41: goto 44 44: areturn Exception table: from to target type 8 20 23 Class scala/runtime/NonLocalReturnControl ``` ``` ⚡ ~/scala/2.11.8/bin/scala Welcome to Scala 2.11.8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_112). Type in expressions for evaluation. Or try :help. scala> @inline def foo(a: => Any) = if ("".isEmpty) a else "" foo: (a: => Any)Any scala> class InlineReturn { def test: Any = foo(return "") } defined class InlineReturn scala> :javap -c InlineReturn#test public java.lang.Object test(); Code: 0: new #4 // class java/lang/Object 3: dup 4: invokespecial #13 // Method java/lang/Object."<init>":()V 7: astore_1 8: getstatic #19 // Field .MODULE$:L; 11: new #21 // class InlineReturn$$anonfun$test$1 14: dup 15: aload_0 16: aload_1 17: invokespecial #24 // Method InlineReturn$$anonfun$test$1."<init>":(LInlineReturn;Ljava/lang/Object;)V 20: invokevirtual #28 // Method .foo:(Lscala/Function0;)Ljava/lang/Object; 23: goto 39 26: astore_2 27: aload_2 28: invokevirtual #31 // Method scala/runtime/NonLocalReturnControl.key:()Ljava/lang/Object; 31: aload_1 32: if_acmpne 40 35: aload_2 36: invokevirtual #34 // Method scala/runtime/NonLocalReturnControl.value:()Ljava/lang/Object; 39: areturn 40: aload_2 41: athrow Exception table: from to target type 8 26 26 Class scala/runtime/NonLocalReturnControl scala> :quit ```
Example usage:
Which gives the following error:
Better than what was previously given: