Skip to content

Commit

Permalink
Eta expansion uses local owner for binders
Browse files Browse the repository at this point in the history
Before, they kept their original owner (the eta expanded class),
which caused an owner-structure mismatch in pickling.
This resulted in spurious recompilation and asSeenFrom crashes.

Changing the owner of the binders used for eta-expansion,
weirdly revealed a problem with typing annotations,
where, I assume, reuse of the class type params hid
the missing logic for dealing with polymorphic annotation
classes and the undetparams that arise of typing their
instantiation.
  • Loading branch information
adriaanm committed Aug 23, 2018
1 parent 9d8f485 commit 6c20b59
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 20 deletions.
19 changes: 10 additions & 9 deletions src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala
Expand Up @@ -96,15 +96,16 @@ abstract class Pickler extends SubComponent {
private def isRootSym(sym: Symbol) =
sym.name.toTermName == rootName && sym.owner == rootOwner

/** Returns usually symbol's owner, but picks classfile root instead
* for existentially bound variables that have a non-local owner.
* Question: Should this be done for refinement class symbols as well?
*
* Note: tree pickling also finds its way here; e.g. in scala/bug#7501 the pickling
* of trees in annotation arguments considers the parameter symbol of a method
* called in such a tree as "local". The condition `sym.isValueParameter` was
* added to fix that bug, but there may be a better way.
*/
/** Usually `sym.owner`, except when `sym` is pickle-local, while `sym.owner` is not.
*
* In the latter case, the alternative owner is the pickle root,
* or a non-class owner of root (so that term-owned parameters remain term-owned).
*
* Note: tree pickling also finds its way here; e.g. in scala/bug#7501 the pickling
* of trees in annotation arguments considers the parameter symbol of a method
* called in such a tree as "local". The condition `sym.isValueParameter` was
* added to fix that bug, but there may be a better way.
*/
private def localizedOwner(sym: Symbol) =
if (isLocalToPickle(sym) && !isRootSym(sym) && !isLocalToPickle(sym.owner))
// don't use a class as the localized owner for type parameters that are not owned by a class: those are not instantiated by asSeenFrom
Expand Down
15 changes: 9 additions & 6 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -3741,18 +3741,20 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
typedFun0
)
val treeInfo.Applied(typedFun @ Select(New(annTpt), _), _, _) = typedFunPart
val annType = annTpt.tpe
val isJava = annType != null && annType.typeSymbol.isJavaDefined
val annType = annTpt.tpe // for a polymorphic annotation class, this type will have unbound type params (see context.undetparams)
val annTypeSym = annType.typeSymbol
val isJava = annType != null && annTypeSym.isJavaDefined

finish(
if (typedFun.isErroneous || annType == null)
ErroneousAnnotation
else if (isJava || annType.typeSymbol.isNonBottomSubClass(ConstantAnnotationClass)) {
else if (isJava || annTypeSym.isNonBottomSubClass(ConstantAnnotationClass)) {
// Arguments of Java annotations and ConstantAnnotations are checked to be constants and
// stored in the `assocs` field of the resulting AnnotationInfo
if (argss.length > 1) {
reportAnnotationError(MultipleArgumentListForAnnotationError(ann))
} else {
// TODO: annType may have undetermined type params -- can we infer them?
val annScopeJava =
if (isJava) annType.decls.filter(sym => sym.isMethod && !sym.isConstructor && sym.isJavaDefined)
else EmptyScope // annScopeJava is only used if isJava
Expand Down Expand Up @@ -3801,11 +3803,12 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
val typedAnn: Tree = {
// local dummy fixes scala/bug#5544
val localTyper = newTyper(context.make(ann, context.owner.newLocalDummy(ann.pos)))
localTyper.typed(ann, mode, annType)
localTyper.typed(ann, mode, deriveTypeWithWildcards(context.undetparams)(annType))
}
def annInfo(t: Tree): AnnotationInfo = t match {
case Apply(Select(New(tpt), nme.CONSTRUCTOR), args) =>
AnnotationInfo(annType, args, List()).setOriginal(typedAnn).setPos(t.pos)
// `tpt.tpe` is more precise than `annType`, since it incorporates the types of `args`
AnnotationInfo(tpt.tpe, args, List()).setOriginal(typedAnn).setPos(t.pos)

case Block(stats, expr) =>
context.warning(t.pos, "Usage of named or default arguments transformed this annotation\n"+
Expand All @@ -3823,7 +3826,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
reportAnnotationError(UnexpectedTreeAnnotationError(t, typedAnn))
}

if (annType.typeSymbol == DeprecatedAttr && argss.flatten.size < 2)
if (annTypeSym == DeprecatedAttr && argss.flatten.size < 2)
context.deprecationWarning(ann.pos, DeprecatedAttr, "@deprecated now takes two arguments; see the scaladoc.", "2.11.0")

if ((typedAnn.tpe == null) || typedAnn.tpe.isErroneous) ErroneousAnnotation
Expand Down
15 changes: 13 additions & 2 deletions src/reflect/scala/reflect/internal/Types.scala
Expand Up @@ -1943,6 +1943,7 @@ trait Types
// (this can happen only for erroneous programs).
}

// TODO should we pull this out to reduce memory footprint of ClassInfoType?
private object enterRefs extends TypeMap {
private var tparam: Symbol = _

Expand Down Expand Up @@ -2496,13 +2497,23 @@ trait Types
val tpars = initializedTypeParams
if (tpars.isEmpty) this
else {
// It's not clear which owner we should use (we don't know the context we're in),
// but pos/t10762 shows it can't be the class (`sym`) that owns the type params,
// as that will confuse ASF during separate compilation.
//
// During pickling, a pickle-local symbol (the type param) that has a non-pickle-local owner (the class),
// will get a new owner (the pickle root, a class) assigned to it by localizedOwner.
// This causes spurious recompilation, as well as confusion in ASF.
// Thus, use a pickle-local term symbol owner and avoid this whole owner-rejiggering.
val pickleLocalOwner = sym.newLocalDummy(sym.pos)

// Since we're going to lose the information denoted by the prefix when pulling the type params
// out for use as binders in the PolyType, we must eagerly rewrite their infos using relativize
// to preserve that knowledge.
val denotedTpars = cloneSymbolsAndModify(tpars, relativize)
val denotedLocallyOwnedTpars = cloneSymbolsAtOwnerAndModify(tpars, pickleLocalOwner, relativize)

// @PP: use typeConstructor! #3343, #4018, #4347.
PolyType(denotedTpars, TypeRef(pre, sym, denotedTpars map (_.typeConstructor)))
PolyType(denotedLocallyOwnedTpars, TypeRef(pre, sym, denotedLocallyOwnedTpars map (_.typeConstructor)))
}
}

Expand Down
6 changes: 3 additions & 3 deletions test/files/neg/hk-typevar-unification.check
@@ -1,6 +1,6 @@
hk-typevar-unification.scala:16: error: inferred kinds of the type arguments ([_ <: B]Foo[_]) do not conform to the expected kinds of the type parameters (type F).
[_ <: B]Foo[_]'s type parameters do not match type F's expected parameters:
type _ (in class Foo)'s bounds <: B are stricter than type _'s declared bounds >: Nothing <: Any
type _'s bounds <: B are stricter than type _'s declared bounds >: Nothing <: Any
f(tcFoo)
^
hk-typevar-unification.scala:16: error: type mismatch;
Expand All @@ -10,8 +10,8 @@ hk-typevar-unification.scala:16: error: type mismatch;
^
hk-typevar-unification.scala:19: error: inferred kinds of the type arguments ([_ <: B]Foo[_]) do not conform to the expected kinds of the type parameters (type F).
[_ <: B]Foo[_]'s type parameters do not match type F's expected parameters:
type _ (in class Foo) is invariant, but type _ is declared covariant
type _ (in class Foo)'s bounds <: B are stricter than type _'s declared bounds >: Nothing <: Any
type _ is invariant, but type _ is declared covariant
type _'s bounds <: B are stricter than type _'s declared bounds >: Nothing <: Any
g(tcFoo)
^
hk-typevar-unification.scala:19: error: type mismatch;
Expand Down
12 changes: 12 additions & 0 deletions test/files/pos/t10762/t10762_1.scala
@@ -0,0 +1,12 @@
import language.higherKinds

trait Tycon[A]
trait MatcherFactory1X[TC1[_]]
trait Matcher[T]{
class X {
def be = {
def inferTycon[TC1[_]](other: MatcherFactory1X[TC1]): MatcherFactory1X[TC1] = ???
inferTycon(??? : MatcherFactory1X[Tycon])
}
}
}
4 changes: 4 additions & 0 deletions test/files/pos/t10762/t10762_2.scala
@@ -0,0 +1,4 @@
object Test {
val m: Matcher[_] = ???
val tpinfer = (new m.X).be
}

0 comments on commit 6c20b59

Please sign in to comment.