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

Require type ascriptions on implicits #11

Closed
puffnfresh opened this issue Sep 4, 2014 · 10 comments
Closed

Require type ascriptions on implicits #11

puffnfresh opened this issue Sep 4, 2014 · 10 comments
Assignees
Labels

Comments

@puffnfresh
Copy link

Jon's anonymous implicit idea also made me think that we should require types on implicits.

i.e. this is invalid:

implicit val i = 0

This is valid:

implicit val i: Int = 0
@puffnfresh puffnfresh added the bug label Sep 4, 2014
@jdegoes
Copy link

jdegoes commented Sep 4, 2014

This is also slated for Typesafe Scala (don't know the ticket).

@puffnfresh
Copy link
Author

@jdegoes http://scala-lang.org/news/roadmap-next mentions it:

Result types are mandatory for implicit definitions.

For the Don Giovanni release (estimated to be released in 2018).

@non
Copy link

non commented Sep 4, 2014

Right now I think this might be a problem for some people, where it's very-hard-or-impossible to explicitly write the type that would be arrived at through inference (and which one needs).

@milessabin is this right? Or am I making this up? I seem to remember someone mentioning it.

@tixxit
Copy link

tixxit commented Sep 4, 2014

@non I had originally used the CaseBuilder return type from Shapeless as an example of a type that would be tedious to type (vs. Shapeless's DSL which will infer the type), but @milessabin said he'd still be happy to force type ascriptions.

@non
Copy link

non commented Sep 4, 2014

Alright. It certainly gets a 👍 from me personally!

@puffnfresh
Copy link
Author

It'd have to be behind a compiler flag to maintain source compatibility anyway, so tricky type users do not have to use the flag.

@paulp
Copy link

paulp commented Sep 5, 2014

I think this would lead to all kinds of busywork, and I don't consider myself an especially "tricky" type creator.

implicit def oot[R: Direct](xs: R) = xs.foo

There's a nice straightforward function, with a type which cannot even be written down because there's no stable path through which to reach it. So you have to unroll the context bound, invent a meaningless name, look up what the type member is called, and now that's a few more bits you get to change in multiple places. Thanks, Compiler.

implicit def oot[R](xs: R)(implicit tc: Direct[R]): tc.Member = xs.foo

There are a bunch of variations on this. I thought the idea was to make it easier to use types to accomplish things, not to exclude concise syntax and make me repeat everything three times, java style.

I'm super well aware of the problem people are trying to address by requiring explicit types, but shifting busywork onto the humans is giving up. There are three benefits to writing the type down which I can think of, other than the artifact-of-the-implementation implicit visibility issue, which I'll ignore here because it's a fairly ridiculous ad hoc "maybe this will work" rule. Maybe I'm overlooking other benefits.

  1. It documents what it is.
  2. It excludes undesirable types from being inferred (since nothing will be inferred.)
  3. It prevents the type from drifting unintentionally.

Writing all the types down is a rigid and costly way to achieve these things. I know how much friction it creates because I already try to write all my implicit return types down (since I lack the mechanisms I'm about to describe) and it's a fuck-ton of friction.

I propose these are better.

  1. Record all inferred types.
  2. Allow users to influence type inference, especially veto. You already do this in wartremover.
  3. Lock inferred types to the recorded ones at well-defined points (say, when the tests pass) and notifying the user when an inferred type moves - especially if the type moved, but the expression didn't.

@Blaisorblade
Copy link

This would need some tool support. In the end, you still want type inference, but it doesn't need to be run in the compiler but in the "IDE" (or equivalent), or the tool @paulp is proposing.

I think this would lead to all kinds of busywork [...]
I thought the idea was to make it easier to use types to accomplish things, not to exclude concise syntax and make me repeat everything three times, java style.

In Haskell, at least you get to write down the function, ask for its type and paste it in later.
Signatures are often recommended for all top-level functions.
And even there, you can get unwanted signatures — say, overly general (though the case I remember was misusing typeclasses).

But I did see code where I decided against adding the signatures.

Lock inferred types to the recorded ones at well-defined points (say, when the tests pass) and notifying the user when an inferred type moves - especially if the type moved, but the expression didn't.

That's a rather nice idea, and I think you might want that for public APIs anyway. A tool for that should have an option for limiting it to implicits, maybe even by default, but that policy choice shouldn't be imposed by the tool.

Could MIMA be configured or patched for that, so that it ignores forward compatibility?
However, that works only if the freeze-points are releases. One alternative is what Scalameter regression testing does: it records locally performance results to check if you've become worse. However, locally is the keyword here.

(I think this mechanism would have to be in a SBT plugin, but the discussion doesn't have a better home than this).

@milessabin
Copy link
Member

@non all implicit result types in shapeless are explicit. I've been advocating this for years, so a definite :+: from me. Needs a flag for backwards compatibility though ... with the flag it should give a deprecation warning I think.

@milessabin
Copy link
Member

Fix in progress as #68.

folone pushed a commit that referenced this issue Aug 14, 2015
Calls to synthetic case class apply methods are inlined to the
underlying constructor invocation in refchecks.

However, this can lead to accessibility errors if the constructor
is private.

This commit ensures that the constructor is at least as accessible
as the apply method before performing this tranform.

I've manually checked that other the optimization still works in other
cases:

scala> class CaseApply { Some(42)  }
defined class CaseApply

    scala> :javap -c CaseApply
    Compiled from "<console>"
    public class CaseApply {
      public CaseApply();
        Code:
           0: aload_0
           1: invokespecial #9                  // Method java/lang/Object."<init>":()V
           4: new           #11                 // class scala/Some
           7: dup
           8: bipush        42
          10: invokestatic  #17                 // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
          13: invokespecial #20                 // Method scala/Some."<init>":(Ljava/lang/Object;)V
          16: pop
          17: return
    }
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

8 participants