Skip to content

Make vals with constant rhs have constant types #4763

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

Closed
wants to merge 2 commits into from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jul 4, 2018

In Scala-2, val's get a constant type only if they are declared final. This is
a horrible wart, of which I was ashamed from the beginning. In Dotty, we definitely do not
want to continue with this tradition. So far, we keep the constant type if the val is
labeled inline. But inline clashes with inline annotations and its unclear whether
we want to keep it.

So I tried to see what breaks if we simply say that val's without a declared
type and a constant rhs get a constant type. No additional modifier needed.
Turns out, a number of small tests break, but they all seem to test this
specific behavior. So it should be easy to change the tests.

The main benefit of the change is:

  • we do not have to introduce another modifier for inlined vals
  • it's shorter and clearer
  • we can always get back the old behavior using an explicit type annotation.

As a next step one could try to avoid widening all singleton types, not
just constant types as in this PR. I have not tried it, since it has no
relevance on the question how to retire final vals.

Note that the situations in Scala-2 and Dotty are different because Dotty
widens at different points. So it could well be that the change is not as
easy in Scala-2.

In Scala-2, val's get a constant type only if they are declared `final`. This is
a horrible wart, of which I was ashamed from the beginning. In Dotty, we definitely do not
want to continue with this tradition. So far, we keep the constant type if the `val` is
labeled `inline`. But `inline` clashes with `inline` annotations and its unclear whether
we want to keep it.

So I tried to see what breaks if we simply say that `val`'s without a declared
type and a constant rhs get a constant type. No additional modifier needed.
Turns out, a number of small tests break, but they all seem to test this
specific behavior. So it should be easy to change the tests.

The main benefit of the change is:

 - we do not have to introduce another modifier for inlined vals
 - it's shorter and clearer
 - we can always get back the old behavior using an explicit type annotation.

As a next step one could try to avoid widening all singleton types, not
just constant types as in this PR. I have not tried it, since it has no
relevance on the question how to retire final vals.

Note that the situations in Scala-2 and Dotty are different because Dotty
widens at different points. So it could well be that the change is not as
easy in Scala-2.
// println(s"final inherited for $sym: ${inherited.toString}") !!!
// println(s"owner = ${sym.owner}, decls = ${sym.owner.info.decls.show}")
def isInline = sym.is(FinalOrInlineOrTransparent, butNot = Method | Mutable)

// Widen rhs type and eliminate `|' but keep ConstantTypes if
// definition is inline (i.e. final in Scala2) and keep module singleton types
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this comment needs to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point

@lrytz
Copy link
Member

lrytz commented Jul 4, 2018

If I understand correctly, this enables inlining / constant folding for all val fields initialized with a literal? I worry about the fact that this breaks separate compilation (if a library changes a val from a literal to a different one, or some other expression, clients need to be re-compiled).

@Blaisorblade
Copy link
Contributor

@lrytz IIUC, indeed — with this change, val x = 1 means val x: 1 = 1, so the body becomes part of the public API, and you can neither override nor replace that with val x = 2, just like today you can't replace val x = 1 with val x = "1". Arguably, this doesn't break separate compilation, it just changes its meaning and makes more changes into source-API breaking.

But I'm not sure that makes things better for those who expect current semantics.

@smarter
Copy link
Member

smarter commented Jul 4, 2018

But I'm not sure that makes things better for those who expect current semantics.

Exactly, this is a silent change of existing semantics, something we should avoid as much as possible to avoid migration (and cross-compilation) headaches. The fact this is currently controlled by the final keyword is weird, but being explicit about it and using inline val or transparent val or const val doesn't seem like a bad thing since we want to encourage modularity anyway (also do we even get any performance benefits from final vals when using Graal?)

@lrytz
Copy link
Member

lrytz commented Jul 5, 2018

this doesn't break separate compilation

I think it does:

$> cat A.scala B.scala
object A {
  final val x = 1
}

object B extends App {
  println(A.x)
}
$> scalac A.scala B.scala
$> cat A.scala
object A {
  final val x = 2
}

$> scalac A.scala
$> scala B
1

@odersky
Copy link
Contributor Author

odersky commented Jul 5, 2018

The point is we want to encourage explicit types anyway. If you foresee a val will be changed to a different constant in the future, write

val x: Int = 2

I think that's overall clearer. I see the migration worries, though.

@Blaisorblade
Copy link
Contributor

The scenario where widening is actually more annoying is when trying to use path-dependent types — which isn't addressed here.

And given our limited test coverage, it seems a bit circular to use it to test impact...

@lrytz I know what you mean, but again technically that's NOT a violation of separate compilation, because otherwise changing from val x: 1 = 1 to val x: 2 = 2 would also violate separate compilation, absurdly. Arguably, it's a change in what it means. Separate compilation means that you can freely change a definition as long as you keep the same public interface, including the types — and the client should still typecheck and need not be recompiled. If the interface uses singleton types, you can't meaningfully change what you return, and for vals, that's the entire implementation.

@lrytz
Copy link
Member

lrytz commented Jul 5, 2018

val x: 1 = 1 to val x: 2 = 2 would also violate separate compilation

Actually it doesn't, at least in the Scala 2.13 implementation (try it out). The implementation makes a difference between FoldableConstantType (constant-folded during type checking) and LiteralType (may be be folded at the type level during type checking, however they will not be folded at the term level).

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Jul 5, 2018

Actually it doesn't, at least in the Scala 2.13 implementation (try it out). The implementation makes a difference between FoldableConstantType (constant-folded during type checking) and LiteralType (may be be folded at the type level during type checking, however they will not be folded at the term level).

@lrytz Uh that's interesting, but then I fear I don't get the point of 2.13's behavior. Let me try to understand — maybe we should replicate it in Dotty and it'd help this PR. My best guess is that this helps people who misunderstand the odd semantics of final and want final val x = 1 to approximate final val x: Int = 1.

A different client, expecting A.x : 1.type, would stop compiling altogether. Contrived example:

object B {
  val y: 1 = A.x
}

So the change in question still requires recompiling clients — and then, what's the use-case for LiteralType?
You've shown it supports changing final val x = 1 to final val x = 2 without recompiling clients. I can imagine some people might want that. But to me that seems as sensible as wanting to change val x = 1 to val x = "1" without recompiling clients. It seems the correct answer is "you needed to write final val x: Int = 1, and maybe the semantics of final are a bit odd".
EDIT: forgot to say: so do you have an answer that improves on my best guess? Or am I suffering from Stockholm-syndrome for the specified but odd behavior?

@milessabin
Copy link
Contributor

Uh that's interesting, but then I fear I don't get the point of 2.13's behavior.

The point is that, for better or worse, we don't want side-effects hidden behind a a literal type to be elided. Eg. in,

// A.scala
object A {
  val x: 1 = { println("Hello world") ; 1 }
}

// B.scala
object B {
  val y = A.x
}

Here the literal typing of A.x should not result in its value being inlined in the RHS of B.y, because if A isn't otherwise touched the side-effect will be lost.

@lrytz
Copy link
Member

lrytz commented Jul 5, 2018

what's the use-case for LiteralType?

A literal-typed term doesn't need to be a compile-time constant:

scala> def f: 1 = {println("hi!"); 1}
f: 1

@lrytz
Copy link
Member

lrytz commented Jul 5, 2018

jinx :)

@liufengyun
Copy link
Contributor

// A.scala
object A {
  val x: 1 = { println("Hello world") ; 1 }
}

// B.scala
object B {
  val y = A.x
}

Here the literal typing of A.x should not result in its value being inlined in the RHS of B.y, because if A isn't otherwise touched the side-effect will be lost.

It seems the problem is more like to be with const-folding, giving the following code:

// A.scala
object A {
  println("Hello world")
  val x: 1 = 1
}

// B.scala
object B {
  val y = A.x
}

We should const-fold A.x as { A; 1 }, that will always be correct, no matter what is in the right-hand side of val x: 1 = rhs.

@milessabin
Copy link
Contributor

We should const-fold A.x as { A; 1 }

Can you say in what sense { A; 1 } is a constant?

@liufengyun
Copy link
Contributor

We should const-fold A.x as { A; 1 }

Can you say in what sense { A; 1 } is a constant?

Indeed it's not a constant in code. But from a broader view of optimisation, it enables further optimisations that are faithful to the original program semantics.

@milessabin
Copy link
Contributor

it enables further optimisations that are faithful to the original program semantics.

I don't quite get it ... if you fold this, you elide the side-effect. That isn't faithful to the original program semantics.

If you can infer that the RHS is pure it would be a different matter.

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Jul 5, 2018

@milessabin You can safely inline (with care) something that isn't constant or has side effects, and I think that's what @liufengyun talks about — instead of cross-module constant folding, we should separate inline and only constant-fold actual constants after checking purity.

So, given

// A.scala
object A {
  println("object init")
  inline def x: 1 = { println("x init"); 1 } //`inline val` requires a val on the rhs.
}

// B.scala
object B {
  val y = A.x + 1
}

Then val y = A.x + 1 should become val y = {A; { println("x init") ; 1 } } + 1 (not what we get, see #4765, but should be "easily" fixed). There, the inlining happens because x is marked inline, not because of the singleton type. For the compiler to constant-fold that and get 2, it'd need first to turn {A; { println("x init") ; 1 } } + 1 into A; println("x init"); 1 + 1 (after checking this is safe) — something which is probably not worth it. Then, constant-folding of terms should only turn 1 + 1 into 2.

Separately, we could type the whole thing as LiteralType(2) maybe, but not as FoldableConstantType(2).

In fact, we could restrict term-level inlining to inline, and that would ease some of @lrytz's concerns — Dotty would just have essentially Scala 2's LiteralType and not FoldableConstantType.

Still, the change in this PR is still problematic.

@lrytz Your examples are a good reason for distinguishing FoldableConstantType and LiteralType. But even with the distinction, replacing val x: 1 = 1 with val x: 2 = 2 is not a legal change under separate compilation because it changes the API and the new program might not be type correct — what is legal is the change between val x: 1 = 1 and val x: 1 = {println(1); 1}.

Erasure and lack of inlining means that if clients compile, they'll produce the same code. But clients might now be ill-typed and ill-behaved. Maybe this is sufficiently rare for primitive literal types (never used them, but users could disagree), but surely not so for singletons in general.

@milessabin
Copy link
Contributor

milessabin commented Jul 5, 2018

@Blaisorblade

Then val y = A.x should become val y = {A; { println("x init") ; 1 } } + 1

(I think you meant val y = A.x + 1)

I don't see that either. Suppose we also had,

// C.scala
object C {
  val z = A.x + 1
}

Mutatis mutandis, val z = A.x + 1 would have to be translated as val z = {A; { println("x init") ; 1 } } + 1. But now the side-effect, which should be performed just once given the usual object initialization semantics, could be performed twice.

@milessabin
Copy link
Contributor

milessabin commented Jul 5, 2018

@Blaisorblade ignore that ... I missed the change from val x to inline def x.

I'm still unclear how this is supposed to interact with object initialization. Is the intention to check object bodies for purity and ban inline definitions if the object body contains any effects?

@Blaisorblade
Copy link
Contributor

(I think you meant val y = A.x + 1)

Fixed thanks!

@Blaisorblade ignore that ... I missed the change from val x to inline def x.

Actually I had missed that too initially, but inline val must be constant expressions (in the sense from the SLS/singleton SIP), so Dotty forced me to use a def. Alternatively, it seems val y = { println("foo"); 1 } could be rewritten to println("foo"); val y = 1, though I'm not sure how that generalizes, I don't know if Dotty does it and I don't advocate it at present.

I'm still unclear how this is supposed to interact with object initialization. Is the intention to check object bodies for purity and ban inline definitions if the object body contains any effects?

#4765 is exactly about that, so I'll answer there.

@odersky
Copy link
Contributor Author

odersky commented Jul 6, 2018

I have the impression this is too controversial to get in.

@odersky odersky closed this Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants