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

Inferring List[Any] #16

Closed
Blaisorblade opened this issue Sep 5, 2014 · 12 comments
Closed

Inferring List[Any] #16

Blaisorblade opened this issue Sep 5, 2014 · 12 comments

Comments

@Blaisorblade
Copy link

While talking about a List(1, "X", 2.0) which was inferred to be List[Any] but was a bug, @paulp wrote in #1:

@Blaisorblade If you have to tell the compiler so the compiler can tell you, I suggest cutting out the middleman.

I see why here (and probably elsewhere) inferring List[Any] is inconvenient.

We should certainly consider a warning for this, and even @paulp's idea of not inferring Any: this is what this issue should be about.

@Blaisorblade
Copy link
Author

I'm uncomfortable with the original suggestion of never inferring Any, because it's irregular, and most Scala irregularities are agreed to be bugs. I'd need to try out a compiler like that before agreeing the idea is good.

What should be the result type of this list? Is it intended to be List[X], or is there a bug?

sealed trait X
case class A(a: Int) extends X
case object B extends X
case object C extends X

List(A(1), B, C)

Similar examples can be repeated at various levels of the hierarchy. You can try to guess what the programmer meant by letting the list members "vote" (so that in List(A(1), A(2), .... A(n), B), B is rejected because the list should be List[A], not List[X]), but that seems a hack.

So in general, you will need some annotations at some point.

This problem arises also because ADTs are encoded by reusing subtyping. So a cleaner design to avoid this problem is to avoid this conflation altogether: ML-like languages have subtyping, but limit it to the module system. I imagine Robert Harper would point this out. I remember also @pchiusano disliked subtyping — is this an argument against subtyping that you like as well, @pchiusano?

I think this difficulty is related with this quote from PFPL:

Subtyping is perhaps the most widely misunderstood concept in programming languages. Subtyping is principally a convenience, akin to type inference, that makes some programs simpler to write. But the subsumption rule cuts both ways. Inasmuch as it allows the implicit passage from τ′ to τ whenever τ′ is a subtype of τ, it also weakens the meaning of a type assertion e : τ to mean that e has some type contained in the type τ. This precludes expressing the requirement that e has exactly the type τ, or that two expressions jointly have the same type. And it is precisely this weakness that creates so many of the difficulties with subtyping.

I agree subtyping is annoying here; but limiting subtyping does have its downsides (and it wouldn't be Scala). So maybe we have to take a non-clean solution for convenience here. But I can't help hoping for some cooler idea here.

Still, a warning for inferred Anys would IMHO be appropriate. It might have to be optional for the sake of -Xfatal-warnings, but support for that is coming.

@paulp
Copy link

paulp commented Sep 5, 2014

You're late to this party:

% scala -Ywarn-infer-any
Welcome to Scala version 2.11.2 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_20).
Type in expressions to have them evaluated.
Type :help for more information.

scala> List(1, "a")
<console>:8: warning: a type was inferred to be `Any`; this may indicate a programming error.
              List(1, "a")
                   ^
res0: List[Any] = List(1, a)

@jdegoes
Copy link

jdegoes commented Sep 5, 2014

I'd love data Foo(...) being shorthand for something like @NonInferrable final case class Foo(...), think it would generally solve the ADT encoding issue without abandoning subtyping, per se.

Also as has been pointed out, not inferring Any doesn't solve any real problems, as Serializable or some other common supertype will be inferred. Don't add more irregularities!

@paulp
Copy link

paulp commented Sep 5, 2014

Any is just the most visible, yes, but the list doesn't go on forever. What good is it to infer Serializable? After Serializable and Product, which we can view as artifacts of the implementation, what type is going to be the lub with any frequency?

@paulp
Copy link

paulp commented Sep 5, 2014

Or maybe you're in the club of AnyVal lovers. Not sure that's a real club.

scala> List(1, true)
<console>:8: warning: a type was inferred to be `AnyVal`; this may indicate a programming error.
              List(1, true)
                   ^
res0: List[AnyVal] = List(1, true)

@jdegoes
Copy link

jdegoes commented Sep 5, 2014

Well, if you don't infer Serializable and Product, you've eliminated some bad cases but more will occur in user-defined type hierarchies (although at least users have control over this to some degree)

To solve it in a user-defined manner, you might need something like "top limit" and "bottom limit" annotations that prevent type inference from inferring a less specific type than something annotated with "top limit", or a more specific type than something annotated with "bottom limit".

@Blaisorblade
Copy link
Author

@NonInferrable

@jdegoes, I love that! We can add syntactic sugar on top, but adding that annotation sounds like a great improvement.

You still need to guess where to put it, and some occurrences are in the standard library, but hey, a library is of course entitled to have ad-hoc code — that's much better than the compiler. And users could add that flag.

Personally, I'm happy with @NonInferrable on all the examples Paul mentioned (Any, AnyVal, Serializable, Product). Per #14, this flag would have to be honored under some -Zhonor-NonInferrable.

To solve it in a user-defined manner, you might need something like "top limit" and "bottom limit" annotations that prevent type inference from inferring a less specific type than something annotated with "top limit", or a more specific type than something annotated with "bottom limit".

I'm not sure yet that's an improvement. Some examples where you need these more complex annotations?

@puffnfresh
Copy link

We should be able to add just an annotation without requiring a flag.

And yes, I really want a NonInferrable annotation.

@jdegoes
Copy link

jdegoes commented Sep 5, 2014

I'm not sure yet that's an improvement. Some examples where you need these more complex annotations?

Bottom limit is equivalent in power to @NonInferrable, but you might use a "top limit" annotation on the trait that unifies the non-inferrable case class children. This would prevent Scala from inferring any supertype of the specified trait.

e.g.

sealed trait Node

@InferrableTop
sealed trait Decl extends Node

@NonInferrable
final case class FnDecl(...) extends Node

@InferrableTop
sealed trait Expr extends Node

@NonInferrable
final case class Mul(...) extends Expr

Thus, things like, List(Mul(...), FnDecl(...)) would be an error, while List(Mul(...)) would be inferred to List[Expr].

@Blaisorblade
Copy link
Author

@jdegoes: Sounds interesting, but let's move to #17 :-).

aloiscochard pushed a commit to aloiscochard/scala that referenced this issue Sep 5, 2014
SI-8345 support both scalap 2.11.0-M8 and 2.11.0-RC1
@stanch
Copy link

stanch commented Sep 13, 2014

I know it’s probably not merge-compatible or fully existing, but what happened to this @paulp? https://groups.google.com/d/topic/scala-language/J8LpYDmrOCg/discussion

val x = List(1, "X", 2.0)
x: List[Int|String|Double] = List(1, X, 2.0)

@paulp
Copy link

paulp commented Sep 13, 2014

No idea. I burned hundreds of branches. My comments in that thread are pretty good I think, and they will have to serve as my legacy.

puffnfresh pushed a commit to puffnfresh/scala that referenced this issue Jul 30, 2015
In Scala 2.8.2, an optimization was added to create a static
cache for Symbol literals (ie, the results of `Symbol.apply("foo"))`.
This saves the map lookup on the second pass through code.

This actually was broken somewhere during the Scala 2.10 series,
after the addition of an overloaded `apply` method to `Symbol`.

The cache synthesis code was made aware of the overload and brought
back to working condition recently, in scala#3149.

However, this has uncovered a latent bug when the Symbol literals are
defined with traits.

One of the enclosed tests failed with:

	  jvm > t8933b-run.log
	java.lang.IllegalAccessError: tried to access field MotherClass.symbol$1 from class MixinWithSymbol$class
	        at MixinWithSymbol$class.symbolFromTrait(A.scala:3)
	        at MotherClass.symbolFromTrait(Test.scala:1)

This commit simply disables the optimization if we are in a trait.
Alternative fixes might be: a) make the static Symbol cache field
public / b) "mixin" the static symbol cache. Neither of these
seem worth the effort and risk for an already fairly situational
optimization.

Here's how the optimization looks in a class:

	% cat sandbox/test.scala; qscalac sandbox/test.scala && echo ":javap C" | qscala;
	class C {
	  'a; 'b
	}
	Welcome to Scala version 2.11.5-20141106-145558-aa558dce6d (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_20).
	Type in expressions to have them evaluated.
	Type :help for more information.

	scala> :javap C
	  Size 722 bytes
	  MD5 checksum 6bb00189166917254e8d40499ee7c887
	  Compiled from "test.scala"
	public class C

	{
	  public static {};
	    descriptor: ()V
	    flags: ACC_PUBLIC, ACC_STATIC
	    Code:
	      stack=2, locals=0, args_size=0
	         0: getstatic     typelevel#16                 // Field scala/Symbol$.MODULE$:Lscala/Symbol$;
	         3: ldc           typelevel#18                 // String a
	         5: invokevirtual typelevel#22                 // Method scala/Symbol$.apply:(Ljava/lang/String;)Lscala/Symbol;
	         8: putstatic     typelevel#26                 // Field symbol$1:Lscala/Symbol;
	        11: getstatic     typelevel#16                 // Field scala/Symbol$.MODULE$:Lscala/Symbol$;
	        14: ldc           typelevel#28                 // String b
	        16: invokevirtual typelevel#22                 // Method scala/Symbol$.apply:(Ljava/lang/String;)Lscala/Symbol;
	        19: putstatic     typelevel#31                 // Field symbol$2:Lscala/Symbol;
	        22: return

	  public C();
	    descriptor: ()V
	    flags: ACC_PUBLIC
	    Code:
	      stack=1, locals=1, args_size=1
	         0: aload_0
	         1: invokespecial typelevel#34                 // Method java/lang/Object."<init>":()V
	         4: getstatic     typelevel#26                 // Field symbol$1:Lscala/Symbol;
	         7: pop
	         8: getstatic     typelevel#31                 // Field symbol$2:Lscala/Symbol;
	        11: pop
	        12: return
	}

fixup
milessabin referenced this issue in milessabin/scala Apr 15, 2016
Rather than in implementation of the abstract method in the
expanded anonymous class.

This leads to more more efficient use of the constant pool,
code shapes more amenable to SAM inlining, and is compatible
with the old behaviour of `-Xexperimental` in Scala 2.11,
which ScalaJS now relies upon.

Manual test:

```
scala> :paste -raw
// Entering paste mode (ctrl-D to finish)

package p1; trait T { val x = 0; def apply(): Any }; class DelambdafyInline { def t: T = (() => "") }

// Exiting paste mode, now interpreting.

scala> :javap -c p1.DelambdafyInline
Compiled from "<pastie>"
public class p1.DelambdafyInline {
  public p1.T t();
    Code:
       0: new           scala#10                 // class p1/DelambdafyInline$$anonfun$t$1
       3: dup
       4: aload_0
       5: invokespecial scala#16                 // Method p1/DelambdafyInline$$anonfun$t$1."<init>":(Lp1/DelambdafyInline;)V
       8: areturn

  public final java.lang.Object p1$DelambdafyInline$$$anonfun$1();
    Code:
       0: ldc           scala#22                 // String
       2: areturn

  public p1.DelambdafyInline();
    Code:
       0: aload_0
       1: invokespecial scala#25                 // Method java/lang/Object."<init>":()V
       4: return
}

scala> :javap -c p1.DelambdafyInline$$anonfun$t$1
Compiled from "<pastie>"
public final class p1.DelambdafyInline$$anonfun$t$1 implements p1.T,scala.Serializable {
  public static final long serialVersionUID;

  public int x();
    Code:
       0: aload_0
       1: getfield      scala#25                 // Field x:I
       4: ireturn

  public void p1$T$_setter_$x_$eq(int);
    Code:
       0: aload_0
       1: iload_1
       2: putfield      scala#25                 // Field x:I
       5: return

  public final java.lang.Object apply();
    Code:
       0: aload_0
       1: getfield      scala#34                 // Field $outer:Lp1/DelambdafyInline;
       4: invokevirtual scala#37                 // Method p1/DelambdafyInline.p1$DelambdafyInline$$$anonfun$1:()Ljava/lang/Object;
       7: areturn

  public p1.DelambdafyInline$$anonfun$t$1(p1.DelambdafyInline);
    Code:
       0: aload_1
       1: ifnonnull     6
       4: aconst_null
       5: athrow
       6: aload_0
       7: aload_1
       8: putfield      scala#34                 // Field $outer:Lp1/DelambdafyInline;
      11: aload_0
      12: invokespecial scala#42                 // Method java/lang/Object."<init>":()V
      15: aload_0
      16: invokespecial scala#45                 // Method p1/T.$init$:()V
      19: return
}

scala> :quit
```

Adriaan is to `git blame` for `reflection-mem-typecheck.scala`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants