Transform/extension methods #140
Merged
Conversation
This file contains hidden or 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
| val forwarder = ref(extensionMeth.termRef) | ||
| .appliedToTypes(origTParams.map(_.typeRef)) | ||
| .appliedToArg(This(origClass)) | ||
| .appliedToArgss(vparamss.nestedMap(vparam => ref(vparam.symbol))) |
There was a problem hiding this comment.
Does this handle repeated parameters? This is handled in Scala 2 with:
// TreeGen
def paramToArg(vparam: Symbol): Tree =
paramToArg(Ident(vparam), isRepeatedParamType(vparam.tpe))
def paramToArg(vparam: ValDef): Tree =
paramToArg(Ident(vparam.name), treeInfo.isRepeatedParamType(vparam.tpt))
def paramToArg(arg: Ident, isRepeatedParam: Boolean): Tree =
if (isRepeatedParam) wildcardStar(arg) else arg
/** Make forwarder to method `target`, passing all parameters in `params` */
def mkForwarder(target: Tree, vparamss: List[List[Symbol]]) =
(target /: vparamss)((fn, vparams) => Apply(fn, vparams map paramToArg))There was a problem hiding this comment.
Repeated parameters are already eliminated in Typer. See makeVarArg in Applications.scala.
There was a problem hiding this comment.
I'm not sure how code in the typer would kick in for the body of this generated forwarder.
Here's the test case I was trying to run:
class Testable(val c: String) extends AnyVal {
def matching(cases: Boolean*) = cases contains true
}
object Test extends App {
assert(new Testable("").matching(true, false))
}(Here's the related bug fix in the old impl: scala/scala@1e0a30a)
I patched a few things, but didn't quite get through the intervening problems:
diff --git a/bin/dotc b/bin/dotc
index eaaf9c7..11df533 100755
--- a/bin/dotc
+++ b/bin/dotc
@@ -6,3 +6,3 @@
# Configuration
-SCALA_VERSION=2.11.0-M7
+SCALA_VERSION=2.11.0-RC3
DOTTY_VERSION=0.1
diff --git a/src/dotty/tools/dotc/core/Symbols.scala b/src/dotty/tools/dotc/core/Symbols.scala
index 6421018..bd82c58 100644
--- a/src/dotty/tools/dotc/core/Symbols.scala
+++ b/src/dotty/tools/dotc/core/Symbols.scala
@@ -393,3 +393,2 @@ object Symbols {
final def isDerivedValueClass(implicit ctx: Context): Boolean =
- false && // value classes are not supported yet
isClass && denot.derivesFrom(defn.AnyValClass) && !isPrimitiveValueClass
@@ -465,3 +464,3 @@ object Symbols {
if (lastDenot == null) s"Naked$prefixString#$id"
- else lastDenot.toString// +"#"+id // !!! DEBUG
+ else lastDenot.toString +"#"+id // !!! DEBUG
diff --git a/src/dotty/tools/dotc/core/transform/Erasure.scala b/src/dotty/tools/dotc/core/transform/Erasure.scala
index e35cdd1..e7c544a 100644
--- a/src/dotty/tools/dotc/core/transform/Erasure.scala
+++ b/src/dotty/tools/dotc/core/transform/Erasure.scala
@@ -163,3 +163,4 @@ class Erasure(isJava: Boolean, isSemi: Boolean, isConstructor: Boolean, wildcard
private def eraseDerivedValueClassRef(tref: TypeRef)(implicit ctx: Context): Type =
- unsupported("eraseDerivedValueClass")
+ defn.AnyRefType
+// unsupported("eraseDerivedValueClass")
diff --git a/src/dotty/tools/dotc/transform/ExtensionMethods.scala b/src/dotty/tools/dotc/transform/ExtensionMethods.scala
index 55b53ef..3abf90c 100644
--- a/src/dotty/tools/dotc/transform/ExtensionMethods.scala
+++ b/src/dotty/tools/dotc/transform/ExtensionMethods.scala
@@ -57,3 +57,3 @@ class ExtensionMethods extends MacroTransform with IdentityDenotTransformer { th
val alts = decl.alternatives
- val index = alts indexOf imeth
+ val index = alts indexOf imeth.denot
assert(index >= 0, alts+" does not contain "+imeth)
@@ -163,2 +163,3 @@ class ExtensionMethods extends MacroTransform with IdentityDenotTransformer { th
.withPos(rhs.pos)
+ println(forwarder)
cpy.DefDef(tree, mods, name, tparams, vparamss, tpt, forwarder)
Exception in thread "main" java.util.NoSuchElementException: key not found: class Testable#5035
at scala.collection.MapLike$class.default(MapLike.scala:228)
at scala.collection.AbstractMap.default(Map.scala:59)
|
Reviewers: Hold off with the extension methods pull request for a bit. I'll have to do some proper testing first. This should not have been a pull request when it was first sent. I just wanted to check whether Travis build would work (because in a previous commit, it didn't), but pushing to staging should have been enough for that. |
|
ExtensionMethods now works and passes the tests. So, ready for review, I think. @retronym @DarkDimius , want to give it a go? Thanks! |
|
Rebased on master and force pushed |
|
There are 5 tests which fail. These are all neg tailrec tests, and the failures are because tailrec is not run. @DarkDimius: Once @tailrec passes -Ycheck can you re-enable it and push on this branch? |
| import NameOps._ | ||
| import TypeUtils._ | ||
|
|
||
| /** A transformer that provides a convenient way to create companion objects |
|
@DarkDimius Have a look at the latest commit. I believe it contains what you need for making type-correct tailrec methods. |
retronym
referenced
this pull request
in dotty-staging/dotty
Jul 14, 2014
The previous version of ExtensionMethods added extension methods manually, in the companion objetct scope, using enteredAfter. Extension methods therefore had to run before pickling, so that other modules would see the generated extension methods. This is suboptimal, for two reasons; (1) Mixin is very much like extension methods, yet it has to run after pickling. (2) The pickling info gets larger than it has to be. With this commit, extension methods are added as an InfoTransformer. So extension methods now should run after pickling.
|
Will die with |
This is done to streamline changing class denotations in new phases by adding to (or otherwise modifying) their decls scope.
Should do the same with other name-creator/name-test pairs.
Added the following utility methods: - polyDefDef: Create a DefDef given a function that takes type and value parameters and yields a body. - appliedToTypeTrees: Apply function to type arguments ion a TypeApply if arguments are nonempty. - mkAsInstanceOf - ensureConforms: generate a cast if expression has non-conforming type.
Added this case, so that .symbol on a ThisType returns the underlying class.
(1) Make sure ModifierFlags is TermFlags and TypeFlags (2) Shorten private <local> to private[this]; same with protected (3) Print [this] for local symbols in RefinedPrinter
Rewrote SuperAccessors (more to be done; see comments), and added stuff here and there to make it work smoother.
... for the common supertype of MethodType, PolyType, and ExprType. Signed was confusing.
The problem is that when an installAfter completely replaces a previous denotation, a symbol or NamedType might still hang on to that denotation in the cache. We need to enforce that we switch to the new denotation. This is done by setting the replaced denotation's validFor field to Nowhere.
d.T is an access to a structural type member, so rejecting this is OK. Not sure why we compiled this before without warning.
1) We now always generate companion objects for classes. This is done in mini-phase "companions", which also assures that companion-modules appear after companion-classes. 2) PostTyperTransformers is gone; the part which normalizes trees has been rolled into TreeTransform and the part which reordered companion classes and modules is now in Companions. Note: Some tests were deisabled; should be re-enabled by Dmitry where needed.
The problem is that exploration methods are run and phase dependent, whereas Definitions has an implicit context that freezes the period when Definitions ewas created. We should complement this by splitting Definitions into a global and per/run part, but that is independent of the change in this commit.
If a symbol is defined in phases M..N, and that symbol is then accessed in a phase before M, but in a new run, we should not issue a stale symbol error (after all, the symbol is not defined yet). Instead we now return a NoDenotation.
If -uniqid is on, RefinedPrinter now prints unique ids in definitions.
Some fixes to toStatic. Also added a method that transforms a RepeatedType to the corresponding Seq type.
Makes sure there are no references to RepeatedParamClass left in the types. Also added placeholder for ElimLocals, which remains to be written.
TreeTypeMap needs to track all binding occurrences and propagate maps to all usage occurrences. Also, fixed `ref` for prefixless types (these mapped to a SelectFromType tree before, now are mapped to Idents).
Fixed extension methods so that it now runs and passes the build. Also enables ElimRepeated, which is a prerequistite for ExtensionMethods. Exception: Tailrec is currently disabled, because it needs to run before ExtensionMethods but it fails the -Ycheck test. Therefore the current tests skip this phase.
Removed diagnsostics message.
When figuring out which of a number of static methods corresponds to an overloaded dynamic method, we only need to compare signatures. No need to resconstruct the full dynamic type.
Pattern match needs to compare against list of names, not single name. Universal equality bites again... Also fix comment for ElimRepeated.
The previous version of ExtensionMethods added extension methods manually, in the companion objetct scope, using enteredAfter. Extension methods therefore had to run before pickling, so that other modules would see the generated extension methods. This is suboptimal, for two reasons; (1) Mixin is very much like extension methods, yet it has to run after pickling. (2) The pickling info gets larger than it has to be. With this commit, extension methods are added as an InfoTransformer. So extension methods now should run after pickling.
When refined printing a DefTree, if one is after typer, print the symbol's definition plus the rhs of the tree rather than the contents of the tree. Why: After typer, it's always the symbol's contents that should matter. Question: Can we enforce through the types that parts of DefTree reflected by the symbol disappear/are ignored?
Singleton types are normally widened before inferring types, yet they did show up as elements of SeqLiterals.
1) Avoid double transformation of a tree 2) Apply the type map to the type of the tree-mapped tree, instead of to the type of the original tree.
1) Disabled insertApplyOrImplicit - it leads to untyped trees which cause assertion errors down the road. In any case, neither apply methods nor implicits can be inserted when retyping. 2) Avoid copying tree annotations into symbols when retyping.
This acommit allows TailRec to run after ExtensionMethods. This is due to the following changes: 1) Extension methods now "rewire" calls to other extension methods of the same class, so that they go directly to other extension methods instead of to the original method in the class. 2) Tailrec annotations get removed from original method and get added to the extension method instead. With this commit all tests can be re-enabled again.
Methods dealing with fully parameterized versions of classes were pulled from `TypeUtils` and `ExtensionMethods` into `FullyParameterization`. `TypeUtils` is left for possible future use, but is empty right now.
t6574 has a new test where we produce identical code in an if-then-else. This broke the rewiring logic before, and is fixed now. Also, more comments and test cases.
Problem was reported by @DarkDimius. Test case will come in next commit.
In a situation like
class C { this: D =>
}
the effective self type of the class is taken to be D & C.
Why? First, Scala 2.x does it that way. Second, it's needed to make the
FullParameterization transform go though.
Adapt the transformation so that self types are handled correctly.
Can now decide on rewiring on a node-by-nide basis.
|
Force-pushed after rebasing on @DarkDimius TreeTransformer fixes. |
|
Force-pushed after @DarkDimius tree transformer fixes. ElimRepeated is now a tree transform, not a macro transform. |
|
LGTM |
DarkDimius
added a commit
that referenced
this pull request
Jul 17, 2014
Transform/extension methods
WojciechMazur
pushed a commit
to WojciechMazur/dotty
that referenced
this pull request
Mar 19, 2025
Backport "Add test cases project for presentation compiler" to 3.3 LTS
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.
New phase ported: extension methods