Erase arguments of phantom type #2399
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolasstucki
force-pushed
the
implement-phantom-types-part-2
branch
6 times, most recently
from
bacfeba to
de3a1c8
Compare
nicolasstucki
force-pushed
the
implement-phantom-types-part-2
branch
from
d273d89 to
2a25799
Compare
nicolasstucki
force-pushed
the
implement-phantom-types-part-2
branch
from
2a25799 to
e8fb429
Compare
odersky
requested changes
Jun 23, 2017
| * | ||
| * - Parameters/arguments are erased to ErasedPhantom. The next step will remove the parameters | ||
| * from the method definitions and calls (implemented in branch implement-phantom-types-part-2). | ||
| * - Parameters/arguments are erased removed from the function definition/call in `PhantomArgumentEval`. |
| @@ -400,12 +400,15 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean | |||
| case tp: MethodType => | |||
| def paramErasure(tpToErase: Type) = | |||
| erasureFn(tp.isJava, semiEraseVCs, isConstructor, wildcardOK)(tpToErase) | |||
| val formals = tp.paramInfos.mapConserve(paramErasure) | |||
| val (names, formals0) = | |||
| if (tp.paramInfos.forall(!_.isPhantom)) (tp.paramNames, tp.paramInfos) | |||
There was a problem hiding this comment.
!tp.paramInfos.exists(_.isPhantom)
| val formals = tp.paramInfos.mapConserve(paramErasure) | ||
| val (names, formals0) = | ||
| if (tp.paramInfos.forall(!_.isPhantom)) (tp.paramNames, tp.paramInfos) | ||
| else tp.paramNames.iterator.zip(tp.paramInfos.iterator).filterNot(_._2.isPhantom).toList.unzip |
There was a problem hiding this comment.
You can probably drop .iterator.
| @@ -474,7 +477,7 @@ object Erasure extends TypeTestsCasts{ | |||
| .withType(defn.ArrayOf(defn.ObjectType)) | |||
| args0 = bunchedArgs :: Nil | |||
| } | |||
| val args1 = args0.zipWithConserve(mt.paramInfos)(typedExpr) | |||
| val args1 = args0.filterConserve(arg => !wasPhantom(arg.typeOpt)).zipWithConserve(mt.paramInfos)(typedExpr) | |||
| untpd.cpy.Apply(tree)(fun1, args1) withType mt.resultType | |||
There was a problem hiding this comment.
Add a comment why we know that argument.isPhantom if and only if parameter.isPhantomj
| } | ||
|
|
||
| /** Collects all args (and possibly the function) as synthetic vals and replaces them in the tree by the reference to | ||
| * the lifted its val. |
| /* Tree transform */ | ||
|
|
||
| override def transformApply(tree: Apply)(implicit ctx: Context, info: TransformerInfo): Tree = tree.tpe match { | ||
| case _: MethodType => tree // Do the transformation higher in the tree if needed |
| private def transformApplication(tree: Tree, mkTree: (Tree, List[ValDef]) => Tree)(implicit ctx: Context): Tree = tree match { | ||
| case tree: Apply => | ||
| def mkNewApply(newFun: Tree, accSynthVals: List[ValDef]) = { | ||
| val (synthVals, synthValRefs) = tree.args.map(mkSynthVal).unzip |
There was a problem hiding this comment.
insert: .filterNot(tpd.isPureExpr)
| case _ => | ||
| def mkNewBlock(newApply: Tree, synthVals: List[ValDef]) = Block(synthVals, newApply) | ||
| if (!hasPhantomArgs(tree)) tree | ||
| else transformApplication(tree, mkNewBlock) |
There was a problem hiding this comment.
Change logic to:
- If function has impure phantom args, lift out all args using
EtaExpansion.liftApp.
nicolasstucki
force-pushed
the
implement-phantom-types-part-2
branch
from
3802539 to
0975c68
Compare
|
All requested changes have been taken care of. |
odersky
approved these changes
Jul 14, 2017
This commit removes arguments and parameter of phantom type from all applications.
Arguments are evaluated before the application in case they have side effects.
```
def foo(i: Int, p: SomePhantom) = ???
foo(42, { println("hello"); getSomePhantom })
```
becomes
```
def foo(i: Int) = ???
val x$0 = 42
val x$1 = { println("hello"); getSomePhantom }
foo(x$0)
```
Note that now `def foo(i: Int)` and def `foo(i: Int, p: SomePhantom)` erase to the same signature.
Tests for this commit where added in `Extra tests for Phantoms 1`, they where back ported as the semantics and runtime is not affected.
Phantom `var`s where disallowed.
nicolasstucki
force-pushed
the
implement-phantom-types-part-2
branch
from
0975c68 to
975cee5
Compare
|
Rebased |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit removes arguments and parameter of phantom type from all applications.
Arguments are evaluated before the application in case they have side effects.
becomes
Note that now
def foo(i: Int)and deffoo(i: Int, p: SomePhantom)erase to the same signature.Tests for this commit where added in
Extra tests for Phantoms 1, they where back ported as the semantics and runtime is not affected.