Permalink
Browse files

refactors handling of parent types

At the moment parser does too much w.r.t handling of parent types.
It checks whether a parent can have value arguments or not and
more importantly, it synthesizes constructors and super calls.

This approach is fundamentally incompatible with upcoming type macros.
Take for example the following two snippets of code:

  `class C extends A(2)`
  `class D extends A(2) with B(3)`

In the first snippet, `A` might be a type macro, therefore the super call
`A.super(2)` eagerly emitted by the parser might be meaningless. In the
second snippet parser will report an error despite that `B` might be
a type macro which expands into a trait.

Unfortunately we cannot simply augment the parser with the `isTypeMacro`
check. This is because to find out whether an identifier refers to a type
macro, one needs to perform a typecheck, which the parser cannot do.

Therefore we need a deep change in how parent types and constructors
are processed by the compiler, which is implemented in this commit.
  • Loading branch information...
1 parent 85f3202 commit 40063b0009d55ed527bf1625d99a168a8faa4124 @xeno-by xeno-by committed Nov 24, 2012
@@ -82,7 +82,7 @@ trait Trees extends scala.reflect.internal.Trees { self: Global =>
* body
* }
*/
- def Template(parents: List[Tree], self: ValDef, constrMods: Modifiers, vparamss: List[List[ValDef]], argss: List[List[Tree]], body: List[Tree], superPos: Position): Template = {
+ def Template(parents: List[Tree], self: ValDef, constrMods: Modifiers, vparamss: List[List[ValDef]], body: List[Tree], superPos: Position): Template = {
/* Add constructor to template */
// create parameters for <init> as synthetic trees.
@@ -117,9 +117,16 @@ trait Trees extends scala.reflect.internal.Trees { self: Global =>
if (vparamss1.isEmpty || !vparamss1.head.isEmpty && vparamss1.head.head.mods.isImplicit)
vparamss1 = List() :: vparamss1;
val superRef: Tree = atPos(superPos)(gen.mkSuperSelect)
- val superCall = (superRef /: argss) (Apply.apply)
@adriaanm

adriaanm Jul 9, 2013

Owner

How is this useful for more than just type macros?

@xeno-by

xeno-by Jul 9, 2013

Member

Immediately useful? I don't think so. Potentially useful by making parser to do more parsing and less desugaring? Yes.

An interesting side-effect of making this more principled is the removal of a "TODO" a couple dozen lines below - now our trees have become a little bit more precise. Another positive effect is making error messages a bit more precise, as in e.g. test/files/neg/t409.check.

@xeno-by

xeno-by Jul 10, 2013

Member

@lrytz says: "For the pendingSuperCall, I'm not sure - OK, it improves an error message, but for quite some added complexity (tree attachment, non-local transformation)".

@xeno-by

xeno-by Jul 10, 2013

Member

Fair enough, I agree that the refactoring comes with a noticeable trade-off in complexity.

+ val superCall = Apply(superRef, Nil) // we can't know in advance which of the parents will end up as a superclass
+ // this requires knowing which of the parents is a type macro and which is not
+ // and that's something that cannot be found out before typer
+ // (the type macros aren't in the trunk yet, but there is a plan for them to land there soon)
+ // this means that we don't know what will be the arguments of the super call
+ // therefore here we emit a dummy which gets populated when the template is named and typechecked
List(
- atPos(wrappingPos(superPos, lvdefs ::: argss.flatten)) (
+ // TODO: previously this was `wrappingPos(superPos, lvdefs ::: argss.flatten)`
+ // is it going to be a problem that we can no longer include the `argss`?
+ atPos(wrappingPos(superPos, lvdefs)) (
DefDef(constrMods, nme.CONSTRUCTOR, List(), vparamss1, TypeTree(), Block(lvdefs ::: List(superCall), Literal(Constant())))))
}
}
@@ -137,11 +144,10 @@ trait Trees extends scala.reflect.internal.Trees { self: Global =>
* @param constrMods the modifiers for the class constructor, i.e. as in `class C private (...)`
* @param vparamss the value parameters -- if they have symbols they
* should be owned by `sym`
- * @param argss the supercall arguments
* @param body the template statements without primary constructor
* and value parameter fields.
*/
- def ClassDef(sym: Symbol, constrMods: Modifiers, vparamss: List[List[ValDef]], argss: List[List[Tree]], body: List[Tree], superPos: Position): ClassDef = {
+ def ClassDef(sym: Symbol, constrMods: Modifiers, vparamss: List[List[ValDef]], body: List[Tree], superPos: Position): ClassDef = {
// "if they have symbols they should be owned by `sym`"
assert(
mforall(vparamss)(p => (p.symbol eq NoSymbol) || (p.symbol.owner == sym)),
@@ -151,7 +157,7 @@ trait Trees extends scala.reflect.internal.Trees { self: Global =>
ClassDef(sym,
Template(sym.info.parents map TypeTree,
if (sym.thisSym == sym || phase.erasedTypes) emptyValDef else ValDef(sym.thisSym),
- constrMods, vparamss, argss, body, superPos))
+ constrMods, vparamss, body, superPos))
}
// --- subcomponents --------------------------------------------------
@@ -1562,9 +1562,9 @@ self =>
val nstart = in.skipToken()
val npos = r2p(nstart, nstart, in.lastOffset)
val tstart = in.offset
- val (parents, argss, self, stats) = template(isTrait = false)
+ val (parents, self, stats) = template()
val cpos = r2p(tstart, tstart, in.lastOffset max tstart)
- makeNew(parents, self, stats, argss, npos, cpos)
+ makeNew(parents, self, stats, npos, cpos)
case _ =>
syntaxErrorOrIncomplete("illegal start of simple expression", true)
errorTermTree
@@ -2742,20 +2742,17 @@ self =>
* TraitParents ::= AnnotType {with AnnotType}
* }}}
*/
- def templateParents(isTrait: Boolean): (List[Tree], List[List[Tree]]) = {
- val parents = new ListBuffer[Tree] += startAnnotType()
- val argss = (
- // TODO: the insertion of ListOfNil here is where "new Foo" becomes
- // indistinguishable from "new Foo()".
- if (in.token == LPAREN && !isTrait) multipleArgumentExprs()
- else ListOfNil
- )
-
- while (in.token == WITH) {
- in.nextToken()
- parents += startAnnotType()
+ def templateParents(): List[Tree] = {
+ val parents = new ListBuffer[Tree]
+ def readAppliedParent() = {
+ val start = in.offset
+ val parent = startAnnotType()
+ val argss = if (in.token == LPAREN) multipleArgumentExprs() else Nil
+ parents += atPos(start)((parent /: argss)(Apply.apply))
}
- (parents.toList, argss)
+ readAppliedParent()
+ while (in.token == WITH) { in.nextToken(); readAppliedParent() }
+ parents.toList
}
/** {{{
@@ -2765,7 +2762,7 @@ self =>
* EarlyDef ::= Annotations Modifiers PatDef
* }}}
*/
- def template(isTrait: Boolean): (List[Tree], List[List[Tree]], ValDef, List[Tree]) = {
+ def template(): (List[Tree], ValDef, List[Tree]) = {
newLineOptWhenFollowedBy(LBRACE)
if (in.token == LBRACE) {
// @S: pre template body cannot stub like post body can!
@@ -2782,16 +2779,16 @@ self =>
case _ => List()
}
in.nextToken()
- val (parents, argss) = templateParents(isTrait = isTrait)
- val (self1, body1) = templateBodyOpt(traitParentSeen = isTrait)
- (parents, argss, self1, earlyDefs ::: body1)
+ val parents = templateParents()
+ val (self1, body1) = templateBodyOpt(parenMeansSyntaxError = false)
+ (parents, self1, earlyDefs ::: body1)
} else {
- (List(), ListOfNil, self, body)
+ (List(), self, body)
}
} else {
- val (parents, argss) = templateParents(isTrait = isTrait)
- val (self, body) = templateBodyOpt(traitParentSeen = isTrait)
- (parents, argss, self, body)
+ val parents = templateParents()
+ val (self, body) = templateBodyOpt(parenMeansSyntaxError = false)
+ (parents, self, body)
}
}
@@ -2805,15 +2802,15 @@ self =>
* }}}
*/
def templateOpt(mods: Modifiers, name: Name, constrMods: Modifiers, vparamss: List[List[ValDef]], tstart: Int): Template = {
- val (parents0, argss, self, body) = (
+ val (parents0, self, body) = (
if (in.token == EXTENDS || in.token == SUBTYPE && mods.isTrait) {
in.nextToken()
- template(isTrait = mods.isTrait)
+ template()
}
else {
newLineOptWhenFollowedBy(LBRACE)
- val (self, body) = templateBodyOpt(traitParentSeen = false)
- (List(), ListOfNil, self, body)
+ val (self, body) = templateBodyOpt(parenMeansSyntaxError = mods.isTrait || name.isTermName)
+ (List(), self, body)
}
)
def anyrefParents() = {
@@ -2835,7 +2832,7 @@ self =>
if (inScalaRootPackage && ScalaValueClassNames.contains(name))
Template(parents0, self, anyvalConstructor :: body)
else
- Template(anyrefParents, self, constrMods, vparamss, argss, body, o2p(tstart))
+ Template(anyrefParents, self, constrMods, vparamss, body, o2p(tstart))
}
}
@@ -2850,14 +2847,15 @@ self =>
case (self, Nil) => (self, EmptyTree.asList)
case result => result
}
- def templateBodyOpt(traitParentSeen: Boolean): (ValDef, List[Tree]) = {
+ def templateBodyOpt(parenMeansSyntaxError: Boolean): (ValDef, List[Tree]) = {
newLineOptWhenFollowedBy(LBRACE)
if (in.token == LBRACE) {
templateBody(isPre = false)
} else {
- if (in.token == LPAREN)
- syntaxError((if (traitParentSeen) "parents of traits" else "traits or objects")+
- " may not have parameters", true)
+ if (in.token == LPAREN) {
+ if (parenMeansSyntaxError) syntaxError(s"traits or objects may not have parameters", true)
+ else assert(false, "unexpected opening parenthesis")
+ }
(emptyValDef, List())
}
}
@@ -205,28 +205,34 @@ abstract class TreeBuilder {
*/
def makeAnonymousNew(stats: List[Tree]): Tree = {
val stats1 = if (stats.isEmpty) List(Literal(Constant(()))) else stats
- makeNew(Nil, emptyValDef, stats1, ListOfNil, NoPosition, NoPosition)
+ makeNew(Nil, emptyValDef, stats1, NoPosition, NoPosition)
}
/** Create positioned tree representing an object creation <new parents { stats }
* @param npos the position of the new
* @param cpos the position of the anonymous class starting with parents
*/
- def makeNew(parents: List[Tree], self: ValDef, stats: List[Tree], argss: List[List[Tree]],
+ def makeNew(parents: List[Tree], self: ValDef, stats: List[Tree],
npos: Position, cpos: Position): Tree =
if (parents.isEmpty)
- makeNew(List(scalaAnyRefConstr), self, stats, argss, npos, cpos)
- else if (parents.tail.isEmpty && stats.isEmpty)
- atPos(npos union cpos) { New(parents.head, argss) }
- else {
+ makeNew(List(scalaAnyRefConstr), self, stats, npos, cpos)
+ else if (parents.tail.isEmpty && stats.isEmpty) {
+ // `Parsers.template` no longer differentiates tpts and their argss
+ // e.g. `C()` will be represented as a single tree Apply(Ident(C), Nil)
+ // instead of parents = Ident(C), argss = Nil as before
+ // this change works great for things that are actually templates
+ // but in this degenerate case we need to perform postprocessing
+ val app = treeInfo.dissectApplied(parents.head)
+ atPos(npos union cpos) { New(app.callee, app.argss) }
+ } else {
val x = tpnme.ANON_CLASS_NAME
atPos(npos union cpos) {
Block(
List(
atPos(cpos) {
ClassDef(
Modifiers(FINAL), x, Nil,
- Template(parents, self, NoMods, ListOfNil, argss, stats, cpos.focus))
+ Template(parents, self, NoMods, ListOfNil, stats, cpos.focus))
}),
atPos(npos) {
New(
@@ -511,7 +511,6 @@ abstract class Constructors extends Transform with ast.TreeDSL {
sym = closureClass,
constrMods = Modifiers(0),
vparamss = List(List(outerFieldDef)),
- argss = ListOfNil,
body = List(applyMethodDef),
superPos = impl.pos)
}
@@ -267,7 +267,7 @@ abstract class UnCurry extends InfoTransform
localTyper.typedPos(fun.pos) {
Block(
- List(ClassDef(anonClass, NoMods, ListOfNil, ListOfNil, List(applyMethodDef), fun.pos)),
+ List(ClassDef(anonClass, NoMods, ListOfNil, List(applyMethodDef), fun.pos)),
Typed(New(anonClass.tpe), TypeTree(fun.tpe)))
}
@@ -392,7 +392,7 @@ abstract class UnCurry extends InfoTransform
localTyper.typedPos(fun.pos) {
Block(
- List(ClassDef(anonClass, NoMods, ListOfNil, ListOfNil, List(applyOrElseMethodDef, isDefinedAtMethodDef), fun.pos)),
+ List(ClassDef(anonClass, NoMods, ListOfNil, List(applyOrElseMethodDef, isDefinedAtMethodDef), fun.pos)),
Typed(New(anonClass.tpe), TypeTree(fun.tpe)))
}
}
@@ -184,14 +184,18 @@ trait ContextErrors {
}
def ParentTypesError(templ: Template, ex: TypeError) = {
- templ.tpe = null
- issueNormalTypeError(templ, ex.getMessage())
+ templ.tpe = null
+ issueNormalTypeError(templ, ex.getMessage())
+ setError(templ)
}
// additional parentTypes errors
- def ConstrArgsInTraitParentTpeError(arg: Tree, parent: Symbol) =
+ def ConstrArgsInParentWhichIsTraitError(arg: Tree, parent: Symbol) =
issueNormalTypeError(arg, parent + " is a trait; does not take constructor arguments")
+ def ConstrArgsInParentOfTraitError(arg: Tree, parent: Symbol) =
+ issueNormalTypeError(arg, "parents of traits may not have parameters")
+
def MissingTypeArgumentsParentTpeError(supertpt: Tree) =
issueNormalTypeError(supertpt, "missing type arguments")
@@ -1043,9 +1047,6 @@ trait ContextErrors {
def MaxParametersCaseClassError(tree: Tree) =
issueNormalTypeError(tree, "Implementation restriction: case classes cannot have more than " + definitions.MaxFunctionArity + " parameters.")
- def InheritsItselfError(tree: Tree) =
- issueNormalTypeError(tree, tree.tpe.typeSymbol+" inherits itself")
-
def MissingParameterOrValTypeError(vparam: Tree) =
issueNormalTypeError(vparam, "missing parameter type")
@@ -847,13 +847,8 @@ trait Namers extends MethodSynthesis {
private def templateSig(templ: Template): Type = {
val clazz = context.owner
def checkParent(tpt: Tree): Type = {
- val tp = tpt.tpe
- val inheritsSelf = tp.typeSymbol == owner
- if (inheritsSelf)
- InheritsItselfError(tpt)
-
- if (inheritsSelf || tp.isError) AnyRefClass.tpe
- else tp
+ if (tpt.tpe.isError) AnyRefClass.tpe
+ else tpt.tpe
}
val parents = typer.parentTypes(templ) map checkParent
@@ -4,7 +4,29 @@ package typechecker
trait StdAttachments {
self: Analyzer =>
+ import global._
+
+ /** Carries information necessary to expand the host tree.
+ * At times we need to store this info, because macro expansion can be delayed until its targs are inferred.
+ * After a macro application has been successfully expanded, this attachment is destroyed.
+ */
type UnaffiliatedMacroContext = scala.reflect.macros.runtime.Context
type MacroContext = UnaffiliatedMacroContext { val universe: self.global.type }
case class MacroRuntimeAttachment(delayed: Boolean, typerContext: Context, macroContext: Option[MacroContext])
+
+ /** After being synthesized by the parser, primary constructors aren't fully baked yet.
+ * A call to super in such constructors is just a fill-me-in-later dummy resolved later
+ * by `parentTypes`. This attachment coordinates `parentTypes` and `typedTemplate` and
+ * allows them to complete the synthesis.
+ */
+ case class SuperCallArgsAttachment(argss: List[List[Tree]])
+
+ /** Extractor for `SuperCallArgsAttachment`.
+ * Compared with `MacroRuntimeAttachment` this attachment has different a usage pattern,
+ * so it really benefits from a dedicated extractor.
+ */
+ object CarriesSuperCallArgs {
+ def unapply(tree: Tree): Option[List[List[Tree]]] =
+ tree.attachments.get[SuperCallArgsAttachment] collect { case SuperCallArgsAttachment(argss) => argss }
+ }
}
Oops, something went wrong.

4 comments on commit 40063b0

Member

xeno-by replied Jul 9, 2013

@adriaanm Could you elaborate on the problems caused by this change? Back then it seemed a good idea even outside macros, because it brought us closer to the better world in which the parser doesn't do desugarings.

Owner

adriaanm replied Jul 9, 2013

See my comment at 40063b0#L0L120

Member

xeno-by replied Jul 9, 2013

See my answer over there as well.

Member

xeno-by replied Jul 10, 2013

Also could you explain the problems that this commit brought to typer?

Please sign in to comment.