Skip to content
This repository was archived by the owner on Sep 1, 2020. It is now read-only.

Added -Zexplicit-implicit-type-ascriptions #68

Closed
wants to merge 20 commits into from
Closed

Added -Zexplicit-implicit-type-ascriptions #68

wants to merge 20 commits into from

Conversation

propensive
Copy link

With the flag enabled, every implicit val/var/def definition must have
an explicit type ascription, otherwise an error is emitted.

non and others added 20 commits September 3, 2014 13:01
This commit reserves the [zZ] and [sS] suffixes for Byte and
Short literals, respectively. These work exactly like Long
literals, which were already denoted with [lL].

For example, you can say:

  val zero = 0Z
  val bytes = List(23Z, 23Z, 0Z, 0Z, 111Z, -54Z, 19Z, 17Z)

This commit does not permit literal unsigned bytes, so 255Z and
0xffZ are not legal.

The choice of Z may seem strange, but B will not work for
hex constants (0x10B would be ambiguous) so Z seemed like a
good choice.
This adds a flag called `-Zirrefutable-generator-patterns` which changes
desugaring of code like the following:

    for {
      (a, b) <- List(1 -> 'a', 2 -> 'b')
      (c, d) <- List(3 -> 'c', 4 -> 'd')
    } yield (a + c, b + d)

Which previously would turn into something like:

    List(1 -> 'a', 2 -> 'b').withFilter {
      case (a, b) => true
      case _ => false
    }.flatMap {
      case (a, b) =>
        List(3 -> 'c', 4 -> 'd').withFilter {
          case (c, d) => true
          case _ => false
        }.map {
          case (c, d) =>
            (a + c, b + d)
        }
    }

With this new flag, it only becomes:

    List(1 -> 'a', 2 -> 'b').flatMap {
      case (a, b) =>
        List(3 -> 'c', 4 -> 'd').map {
          case (c, d) =>
            (a + c, b + d)
        }
    }

Which creates a few benefits:

1. We don't have to do a useless call to withFilter
2. We can use patterns on things without withFilter
3. Things don't silently disappear when patterns are wrong
We assume patterns are exhaustive when desugaring with
`-Zirrefutable-generator-patterns` but we should still emit a warning if
we think they might not be.
Add support for literal Byte and Short values.
Allow primes at the end of identifiers.
…tterns

Conflicts:
	bincompat-forward.whitelist.conf
	src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
…atterns

Add flag to disable withFilter pattern desugaring
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]
                ^
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.
…otation

Add an @implicitAmbiguous annotation
With the flag enabled, every implicit val/var/def definition must have
an explicit type ascription, otherwise an error is emitted.
@puffnfresh
Copy link

Do the type ascription changes to the compiler and stdlib need to be made? They won't have the flag enabled, right?

@propensive
Copy link
Author

Flag's not enabled, so they're not required, no.

But they are probably a good thing, and the work's been done, so I've left them in, as the binary compatibility tests all pass.

@Blaisorblade
Copy link

Why not enable the flag then? Otherwise this change will not be maintained.

@puffnfresh
Copy link

My minor concern is this could make merging Typesafe scalac a little annoying but probably not.

@retronym
Copy link

retronym commented Oct 2, 2014

srewrite, an experimental source code migration tool, supports automatically adding type annotations to implicits. @odersky would like to move to this policy in dotc, so we're quite happy to have a lint warning for non-annotated implicits over in scala/scala sooner rather than later.

@retronym
Copy link

retronym commented Oct 2, 2014

Oh, and we'd definitely appreciate the patch with the explicit annotations throughout our code!

@som-snytt
Copy link

So I just wrote

  import scala.io.Codec.UTF8
  implicit def `we love utf8` = UTF8

and obviously I don't want to add an import so I can write : Codec. Isn't there an implicit import PR floating around somewhere that makes this angle less annoying?

This was in a test, BTW, also subject to migration.

@propensive
Copy link
Author

@retronym The patch should ascribe explicit types to all the instances that flagged as errors during compilation of the compiler and standard library, with one exception (which unfortunately I haven't been able to find in ten minutes of trying to find it again): there's a method somewhere which returns an implicit def which returns an instance of a class defined locally to that method. I think this means the implicit will get a structural type inferred, but it was sufficiently horrible that I stopped there...

@propensive
Copy link
Author

@som-snytt I'm not sure I'd want an implicit import for such a specific use case, though I agree it's annoying. Suggestions welcome...

@milessabin
Copy link
Member

If you would like to resurrect this issue/PR please rework it as a PR against Lightbend Scala 2.12.x (ie. scala/scala).

@milessabin milessabin added this to the Parked milestone Aug 12, 2016
@milessabin milessabin closed this Aug 26, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants