Decouple transparent and erased properties #4860

odersky · 2018-07-28T14:52:09Z

It is at times useful to have transparent methods that are not
erased. For instance, a transparent unapply might enable inline
reductions, but we want to also keep it at runtime, because the same
type might be matched at compile-time and at run-time.

This commit decouples transparent and erased again. transparent
does not imply erased.

odersky · 2018-07-28T17:48:29Z

I am still not sure about this. It seems we need two kinds of transparent methods:

Compile-time only, i.e. transparent and erased. These methods

are inlined
don't generate code
allow implicit matches
allow macro splices
don't inline in their body before they are expanded themselves (this means recursion can be expressed safely)
cannot override other methods or they have a special semantics for it

Compile-time + Run-time, i.e. transparent but not erased. These methods:

are also inlined, but
do generate code
don't allow implicit matches or macro splices
do inline in their body before they are expanded themselves - this means recursion is basically impossible, since at the definition site there is usually not enough knowledge to prune a recursive call, which means a recursive method will expand until the limit is exceeded.
can override other methods

Examples of the first kind of method are all the typelevel functions discussed in #4616. An example of the second kind of method is a transparent unapply that needs to be inlined so that a toplevel match can "see through it" but that also should be retained at runtime to allow normal matching.

In this PR, the first kind of method is labelled erased transparent, the second just transparent. But erased transparent is a mouthful. Other possibilities include:

macro for the first kind, transparent for the second
transparent for the first kind, translucent for the second

Any other ideas?

Note: Why do the second kind of methods inline in their bodies before they are expanded? Because these calls might go to erased transparent functions, which will not be available at runtime. An alternative, (which is what's implemented in 1299850) is to not inline in the body, but then erased transparency becomes transitive. You cannot call an erased transparent function from a normal transparent function, since the call does not go away. This seems weird. After all, you can call an erased transparent function from a normal function, since the call simply gets inlined there.

odersky · 2018-07-28T20:38:37Z

I discovered a tweak which, even though it is very small, might change the balance of power. The tweak is:

When processing the body of a transparent, unerased method we inline only transparent, erased methods. Transparent, unerased methods are not inlined, since they do have a runtime representation.

That means we essentially have two modes:

If a transparent, unerased method is inlined, all calls to transparent methods are inlined in turn.
But if it is left to be executed at runtime, we inline only the minimum (i.e. the erased methods) whereas all calls to other transparent unerased methods are run-time calls.

This means that now we don't have to choose: transparent methods are fine even if they are not erased. They can do everything transparent erased methods can do, as long as they do not contain implicit matches or macro splices. So there is a clear criterion when to go to transparent erased, and, furthermore, it's a criterion that won't trigger too often.

Let's redo the characterisation with this change:

transparent erased methods

are inlined
don't generate code
allow implicit matches
allow macro splices
don't inline in their body before they are expanded themselves
cannot override other methods or they have a special semantics for it

transparent methods

are also inlined, but
do generate code
don't allow implicit matches or macro splices
do inline erased transparents in their body before they are expanded themselves
can override other methods

odersky · 2018-07-29T18:10:14Z

@nicolasstucki I believe the whole Splicer.canbeSpliced logic can be eliminated, including the overhead of defining an abstract interpreter. A transparent method is marked as a macro (now already in Namer/Typer) if it contains a toplevel splice. Macros process their right hand side, which is known to be a splice. Other splices outside quotes are errors. That should be it. We don't need a separate testing logic here.

nicolasstucki · 2018-07-30T06:38:35Z

To allow the use of erased transparent in a transparent we also should not allow the transparent (un-erased) method to have transparent parameters. Otherwise, when inlining the erased transparent in the transparent method body the parameter will not be a literal constant. This would cause problems for macros.

nicolasstucki · 2018-07-30T06:43:36Z

@nicolasstucki I believe the whole Splicer.canbeSpliced logic can be eliminated, including the overhead of defining an abstract interpreter. A transparent method is marked as a macro (now already in Namer/Typer) if it contains a toplevel splice. Macros process their right hand side, which is known to be a splice. Other splices outside quotes are errors. That should be it. We don't need a separate testing logic here.

Splicer.canbeSpliced does not check if the top-level splice is a valid top-level splice. Instead it tests if the contents of a valid top level splice will be interpretable at inline site. Though this new check will help remove some other logic that checks the top-level splices.

nicolasstucki · 2018-07-30T06:44:44Z

compiler/src/dotty/tools/dotc/config/Config.scala

@@ -160,7 +160,7 @@ object Config {
  final val showCompletions = false

  /** If set, enables tracing */
-  final val tracingEnabled = false
+  final val tracingEnabled = true


Was this enabled on purpose?

No, I mistakenly added two files to the commit that should not be there.

nicolasstucki · 2018-07-30T06:45:10Z

compiler/src/dotty/tools/dotc/core/Flags.scala

@@ -548,8 +548,8 @@ object Flags {
  /** Assumed to be pure */
  final val StableOrErased = Stable | Erased

-  /** Labeled `private`, `final`, or `transparent` */
-  final val EffectivelyFinal = Private | Final | Transparent
+  /** Labeled `private`, `final`, `transparent`, or `erasedd` */


Typo erasedd -> erased.

nicolasstucki · 2018-07-30T06:46:13Z

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -761,7 +761,7 @@ class ClassfileParser(
          assert(cond,
            s"Unpickling ${classRoot.symbol.showLocated} from ${classRoot.symbol.associatedFile} is not allowed with -Yscala2-unpickler $allowed")

-        if (allowed != "always") {
+        if (false && allowed != "always") {


Should be removed

Ah, I needed that because my local setup does not run otherwise. I have to inquire why.

nicolasstucki · 2018-07-30T06:55:44Z

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

-      } else if (member.is(Erased) && member.allOverriddenSymbols.forall(_.is(Deferred))) { // (1.9)
-        overrideError("is an erased method, may not override only deferred methods")
+      } else if (member.is(Erased)) { // (1.9)
+        overrideError("is an erased method, may not override anything")
      } else if (member.is(Macro, butNot = Scala2x)) { // (1.9)
        overrideError("is a macro, may not override anything")


Unreachable code. This case is covered be the previous one.

nicolasstucki · 2018-07-30T06:59:42Z

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

@@ -143,7 +143,7 @@ object RefChecks {
   *    1.8.1  M's type is a subtype of O's type, or
   *    1.8.2  M is of type []S, O is of type ()T and S <: T, or
   *    1.8.3  M is of type ()S, O is of type []T and S <: T, or
-   *    1.9    M must not be a typelevel def or a Dotty macro def
+   *    1.9    M must not be an erased definition
   *    1.10.  If M is a 2.x macro def, O cannot be deferred unless there's a concrete method overriding O.
   *    1.11.  If M is not a macro def, O cannot be a macro def.


The 1.11 rule should probably be for erased in general.

In fact, 1.11 is not needed at all, since macros are erased, hence effectively final.

odersky · 2018-07-30T08:18:08Z

To allow the use of erased transparent in a transparent we also should not allow the transparent (un-erased) method to have transparent parameters. Otherwise, when inlining the erased transparent in the transparent method body the parameter will not be a literal constant. This would cause problems for macros.

But macros are always implicitly erased. And I don't think there is a problem for normal transparent functions.

odersky · 2018-07-30T08:41:04Z

The latest commit brings back the rule that erased methods may override other methods, as long as at least one of the overridden methods is concrete. Scala 2 has the same rule for macros. I believe it's a useful rule to have, since otherwise we can never re-implement an inherited method with macro-generated code. We can think about tightening the rule later.

nicolasstucki · 2018-07-30T09:39:52Z

To allow the use of erased transparent in a transparent we also should not allow the transparent (un-erased) method to have transparent parameters. Otherwise, when inlining the erased transparent in the transparent method body the parameter will not be a literal constant. This would cause problems for macros.

But macros are always implicitly erased. And I don't think there is a problem for normal transparent functions.

The issue arises with the combination of both

transparent def foo(transparent i: Int): Int = bar(i)
erased transparent def bar(transparent i: Int): Int = ~baz(i)
def baz(i: Int): Int = i

In this case, for the body of foo (retained at runtime) we will inline bar getting

transparent def foo(transparent i: Int): Int = ~baz(i)

but now we cannot evaluate ~baz(i) as i is still unknown.

It is at times useful to have `transparent` methods that are not `erased`. For instance, a transparent `unapply` might enable inline reductions, but we want to also keep it at runtime, because the same type might be matched at compile-time and at run-time. This commit decouples `transparent` and `erased` again. `transparent` does not imply `erased`.

In bodies of transparent unerased method we inline only transparent erased methods.

- Mark a method with a toplevel splice as a macro already when we register the inline info. This is needed so that such methods are recognized as erased the first time someone wants to inline them. - Improve error message for splice outside quote.

We need the `erased` on foo because otherwise we get the error -------------------------------- 6 | transparent def foo(): Unit = foo2() | ^^^^^^ |Could not find macro class Foo$ in classpath. The most common reason for that is that you cannot use transparent macro implementations in the same compilation run that defines them

nicolasstucki · 2018-08-07T06:41:06Z

Test tests/run/i4803d is failing because the macro power is not marked as erased and the splice survives pruneErasedDefs.

Then there is a second problem, in the body of power2 the method power is not inlined. In general, we should probably inline erased transparent in the RHS of transparent methods.
This issue is not only present due to the nesting, it can also be reproduced if the methods are defined as:

transparent def power2(x: Double) = power(x, 2)
erased transparent def power(x: Double, transparent n: Long) = ~PowerMacro.powerCode('(x), n)

We will probably have the same issues for type level matches in transparent methods.

nicolasstucki · 2018-08-07T08:17:29Z

Deciding if a transparent is erased or not based on the contents of the right-hand side of the method is a bit ad-hoc. I would propose trying another approach that would keep transparent methods based on the override keyword. The idea would be that by default all transparent methods are erased. transparent method would not be erased only if it is a override transparent which implies that it implements a member (concrete or abstract) and is needed at runtime.

The complete rules would be

All transparent are final
All local transparent methods are erased (as they do not override any method)
transparent methods are erased
- may have implicit matches
- may have top-level ~
- all calls must be inlined, (including calls in RHS of override transparent)
- does not implement/override any other method
- internally has the erased flag
override transparent are not erased
- cannot have implicit matches
- cannot have top-level ~
- all calls to the macro should be inlined
- implements/overrides other method

odersky · 2018-08-10T18:10:53Z

Closed for now, in favor of #4927.

odersky added the stat:wip label Jul 28, 2018

odersky requested a review from nicolasstucki July 29, 2018 18:10

odersky removed the stat:wip label Jul 29, 2018

odersky force-pushed the decouple-transparent-erased branch from d655ee3 to 648b8a4 Compare July 29, 2018 18:15

odersky assigned nicolasstucki Jul 29, 2018

nicolasstucki reviewed Jul 30, 2018

View reviewed changes

odersky added 9 commits August 1, 2018 16:15

Implement tweak

09cd3ee

In bodies of transparent unerased method we inline only transparent erased methods.

Change macro labelling

edf98e3

- Mark a method with a toplevel splice as a macro already when we register the inline info. This is needed so that such methods are recognized as erased the first time someone wants to inline them. - Improve error message for splice outside quote.

Revert accidentally committed files

b573150

Fix typo and renaming

25c04af

Fix RefCheck rules

dcdd51d

Make implicit match imply erased

6bf58a5

Fix rebase breakage

370125e

odersky force-pushed the decouple-transparent-erased branch from 95d06bc to 370125e Compare August 1, 2018 14:18

nicolasstucki mentioned this pull request Aug 4, 2018

Overriding a method with a inline method is inconstant #4892

Closed

nicolasstucki assigned odersky and unassigned nicolasstucki Aug 7, 2018

odersky closed this Aug 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Decouple transparent and erased properties #4860

Decouple transparent and erased properties #4860

odersky commented Jul 28, 2018

odersky commented Jul 28, 2018 •

edited

Loading

odersky commented Jul 28, 2018 •

edited

Loading

odersky commented Jul 29, 2018

nicolasstucki commented Jul 30, 2018

nicolasstucki commented Jul 30, 2018

nicolasstucki Jul 30, 2018

odersky Jul 30, 2018

nicolasstucki Jul 30, 2018

nicolasstucki Jul 30, 2018

odersky Jul 30, 2018

nicolasstucki Jul 30, 2018

nicolasstucki Jul 30, 2018

odersky Jul 30, 2018

odersky commented Jul 30, 2018

odersky commented Jul 30, 2018 •

edited

Loading

nicolasstucki commented Jul 30, 2018

nicolasstucki commented Aug 7, 2018

nicolasstucki commented Aug 7, 2018

odersky commented Aug 10, 2018

Decouple transparent and erased properties #4860

Decouple transparent and erased properties #4860

Conversation

odersky commented Jul 28, 2018

odersky commented Jul 28, 2018 • edited Loading

odersky commented Jul 28, 2018 • edited Loading

odersky commented Jul 29, 2018

nicolasstucki commented Jul 30, 2018

nicolasstucki commented Jul 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odersky commented Jul 30, 2018

odersky commented Jul 30, 2018 • edited Loading

nicolasstucki commented Jul 30, 2018

nicolasstucki commented Aug 7, 2018

nicolasstucki commented Aug 7, 2018

odersky commented Aug 10, 2018

odersky commented Jul 28, 2018 •

edited

Loading

odersky commented Jul 28, 2018 •

edited

Loading

odersky commented Jul 30, 2018 •

edited

Loading