Properly desugar inline given .. with ..
#14284
Conversation
|
Replaces #14207 |
nicolasstucki
force-pushed
the
fix-#14282
branch
from
09e7af1 to
ee27ed4
Compare
```scala
inline given Foo with { .. }
```
now becomes
```scala
inline given def given_Foo: Foo with {..} = new Foo
class given_Foo extends Foo with { }
```
There are no creation of `lazy vals` when we have `inline`.
We inline the `new givne_Foo` only, the class does not get inlined.
This is useful to remove the instatiation of the given instance when
the methods are inline themselfs.
Fixes scala#14282
Fixes scala#14177
nicolasstucki
force-pushed
the
fix-#14282
branch
from
ee27ed4 to
9b3cdc2
Compare
There was a problem hiding this comment.
This would need to be documented in the "Given Instance Initialization" section of https://dotty.epfl.ch/docs/reference/contextual/givens.html : If I understand this correctly, if the trait extended by the given instance has side effects in its constructor, they will now be executed at each invocation (we should have tests for that too).
I think this looks reasonable but it's still a spec change so /cc @odersky (and it will need to be in the release notes of a minor version presumably)
| @@ -0,0 +1,15 @@ | |||
| class T | |||
|
|
|||
| inline given fail1: T with | |||
There was a problem hiding this comment.
Does transparent inline work / do anything useful here?
There was a problem hiding this comment.
It makes sure that we do not hit the previous issue where the summonAll was not inlined due to the inline flag that was kept by mistake on the class fail1.
There was a problem hiding this comment.
I mean, there are no testcases with transparent in this pr
There was a problem hiding this comment.
Sorry, I completely missed the transparent. I will add some test cases.
There was a problem hiding this comment.
Transparent is failing for the wrong reason. I do not see how transparent would be useful here because we already have the precise type. I will disallow this combination and make sure the error message is correct.
| inline given Show[Foo] with { | ||
| /*transparent*/ inline def show(x: Foo): String = ${ ... } | ||
| } |
There was a problem hiding this comment.
This example explains inline given but also uses inline on the method definition even though these two concepts are orthogonal, couldn't the example use a non-inline def to be kept simple?
There was a problem hiding this comment.
I could not come up with a useful example of inlining those givens that did not involve also inlining the method.
There was a problem hiding this comment.
Actually, the only reason why we did not disallow inline given .. with .. is for the case where it contains an inline method (as I originally intended in #14207).
There was a problem hiding this comment.
Could the doc describe a concrete situation where it's useful then? I imagine it's needed for the method call to actually be inlined but that should be shown explicitly with an example because it's not obvious
There was a problem hiding this comment.
I imagine it's needed for the method call to actually be inlined but that should be shown explicitly with an example because it's not obvious
The use case of the issue at the origin of this PR was compile-time typeclass. The goal is to allow, as you said, to fully inline the method call. For example, I use it for type refinements/constraints.
now becomes
There are no creation of
lazy valswhen we haveinline.We inline the
new givne_Fooonly, the class does not get inlined.This is useful to remove the instatiation of the given instance when
the methods are inline themselfs.
Fixes #14282
Fixes #14177