From 6136794d23057f60738c0ce523214bc818135a3e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 11 Apr 2015 11:36:56 +0200 Subject: [PATCH 1/9] Merge pull request #469 from dotty-staging/stylecheck Add stylechecking to test suite, fix style errors --- src/dotty/tools/dotc/typer/Checking.scala | 14 +++++++++----- src/dotty/tools/dotc/typer/Typer.scala | 4 ++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/dotty/tools/dotc/typer/Checking.scala b/src/dotty/tools/dotc/typer/Checking.scala index 9303572d2a7c..a547b3f178f1 100644 --- a/src/dotty/tools/dotc/typer/Checking.scala +++ b/src/dotty/tools/dotc/typer/Checking.scala @@ -241,16 +241,20 @@ trait Checking { def checkLegalPrefix(tp: Type, selector: Name, pos: Position)(implicit ctx: Context): Unit = if (!tp.isLegalPrefixFor(selector)) ctx.error(d"$tp is not a valid prefix for '# $selector'", pos) - /** Check that `tp` is a class type with a stable prefix. Also, if `traitReq` is - * true check that `tp` is a trait. + /** Check that `tp` is a class type with a stable prefix. Also: + * If `traitReq` is true, check that `tp` refers to a trait. + * If `concreteReq` is true, check that `tp` refers to a nonAbstract class. * Stability checking is disabled in phases after RefChecks. * @return `tp` itself if it is a class or trait ref, ObjectClass.typeRef if not. */ - def checkClassTypeWithStablePrefix(tp: Type, pos: Position, traitReq: Boolean)(implicit ctx: Context): Type = + def checkClassTypeWithStablePrefix(tp: Type, pos: Position, traitReq: Boolean = false, concreteReq: Boolean = false)(implicit ctx: Context): Type = tp.underlyingClassRef(refinementOK = false) match { case tref: TypeRef => if (ctx.phase <= ctx.refchecksPhase) checkStable(tref.prefix, pos) - if (traitReq && !(tref.symbol is Trait)) ctx.error(d"$tref is not a trait", pos) + if (traitReq && !(tref.symbol is Trait)) + ctx.error(d"$tref is not a trait", pos) + if (concreteReq && false && (tref.symbol is AbstractOrTrait)) + ctx.error(d"${tref.symbol} is abstract; cannot be instantiated, owner = ${ctx.owner}", pos) tp case _ => ctx.error(d"$tp is not a class type", pos) @@ -329,7 +333,7 @@ trait NoChecking extends Checking { override def checkBounds(args: List[tpd.Tree], poly: PolyType)(implicit ctx: Context): Unit = () override def checkStable(tp: Type, pos: Position)(implicit ctx: Context): Unit = () override def checkLegalPrefix(tp: Type, selector: Name, pos: Position)(implicit ctx: Context): Unit = () - override def checkClassTypeWithStablePrefix(tp: Type, pos: Position, traitReq: Boolean)(implicit ctx: Context): Type = tp + override def checkClassTypeWithStablePrefix(tp: Type, pos: Position, traitReq: Boolean, concreteReq: Boolean)(implicit ctx: Context): Type = tp override def checkImplicitParamsNotSingletons(vparamss: List[List[ValDef]])(implicit ctx: Context): Unit = () override def checkFeasible(tp: Type, pos: Position, where: => String = "")(implicit ctx: Context): Type = tp override def checkNoDoubleDefs(cls: Symbol)(implicit ctx: Context): Unit = () diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index a2b280c6e922..cf0762ef01dd 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -347,8 +347,8 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit val clsDef = TypeDef(x, templ).withFlags(Final) typed(cpy.Block(tree)(clsDef :: Nil, New(Ident(x), Nil)), pt) case _ => - val tpt1 = typedType(tree.tpt) - checkClassTypeWithStablePrefix(tpt1.tpe, tpt1.pos, traitReq = false) + val tpt1 = typedType(tree.tpt) + checkClassTypeWithStablePrefix(tpt1.tpe, tpt1.pos, concreteReq = !ctx.owner.isClass) assignType(cpy.New(tree)(tpt1), tpt1) // todo in a later phase: checkInstantiatable(cls, tpt1.pos) } From bf9da749bde9ab3c5469c71a43888c189e0fd98f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 12 Apr 2015 17:55:18 +0200 Subject: [PATCH 2/9] Better printing of anonymous classes. Now prints `C{...}` instead of `$anon`. --- src/dotty/tools/dotc/printing/PlainPrinter.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/printing/PlainPrinter.scala b/src/dotty/tools/dotc/printing/PlainPrinter.scala index 2762d9b51151..0a7edd2aa779 100644 --- a/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -354,7 +354,8 @@ class PlainPrinter(_ctx: Context) extends Printer { def toText(sym: Symbol): Text = (kindString(sym) ~~ { - if (hasMeaninglessName(sym)) simpleNameString(sym.owner) + idString(sym) + if (sym.isAnonymousClass) toText(sym.info.parents, " with ") ~ "{...}" + else if (hasMeaninglessName(sym)) simpleNameString(sym.owner) + idString(sym) else nameString(sym) }).close From 01b585c865d17d24c3066c434e01c84a6c871c4b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 12 Apr 2015 18:01:27 +0200 Subject: [PATCH 3/9] New method on types: givenSelfType The self type as given (or implied for module classes) in the source. Also defined and updated for normal types, not just ClassInfo types. Common functionality between this and baseTypeWithArgs has been pulled into RefinedType#wrapIfMember. --- .../tools/dotc/core/TypeApplications.scala | 6 +-- src/dotty/tools/dotc/core/Types.scala | 45 +++++++++++++------ 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/dotty/tools/dotc/core/TypeApplications.scala b/src/dotty/tools/dotc/core/TypeApplications.scala index df18813b9280..de42b3e5fea5 100644 --- a/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/src/dotty/tools/dotc/core/TypeApplications.scala @@ -268,9 +268,7 @@ class TypeApplications(val self: Type) extends AnyVal { case _ => default } case tp @ RefinedType(parent, name) if !tp.member(name).symbol.is(ExpandedTypeParam) => - val pbase = parent.baseTypeWithArgs(base) - if (pbase.member(name).exists) RefinedType(pbase, name, tp.refinedInfo) - else pbase + tp.wrapIfMember(parent.baseTypeWithArgs(base)) case tp: TermRef => tp.underlying.baseTypeWithArgs(base) case AndType(tp1, tp2) => @@ -281,7 +279,7 @@ class TypeApplications(val self: Type) extends AnyVal { default } } - + /** Translate a type of the form From[T] to To[T], keep other types as they are. * `from` and `to` must be static classes, both with one type parameter, and the same variance. */ diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index e6235695ed8e..fe95219b8415 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -858,6 +858,13 @@ object Types { case _ => defn.AnyClass.typeRef } + /** the self type of the underlying classtype */ + def givenSelfType(implicit ctx: Context): Type = this match { + case tp @ RefinedType(parent, name) => tp.wrapIfMember(parent.givenSelfType) + case tp: TypeProxy => tp.underlying.givenSelfType + case _ => NoType + } + /** The parameter types of a PolyType or MethodType, Empty list for others */ final def paramTypess(implicit ctx: Context): List[List[Type]] = this match { case mt: MethodType => mt.paramTypes :: mt.resultType.paramTypess @@ -1781,7 +1788,12 @@ object Types { if (false) RefinedType(parent, refinedName, refinedInfo) else RefinedType(parent, refinedName, rt => refinedInfo.substSkolem(this, SkolemType(rt))) } - + + /** Add this refinement to `parent`, provided If `refinedName` is a member of `parent`. */ + def wrapIfMember(parent: Type)(implicit ctx: Context): Type = + if (parent.member(refinedName).exists) derivedRefinedType(parent, refinedName, refinedInfo) + else parent + override def equals(that: Any) = that match { case that: RefinedType => this.parent == that.parent && @@ -2398,22 +2410,27 @@ object Types { * - the fully applied reference to the class itself. */ def selfType(implicit ctx: Context): Type = { - if (selfTypeCache == null) { - def fullRef = fullyAppliedRef(cls.typeRef, cls.typeParams) - def withFullRef(tp: Type): Type = - if (ctx.erasedTypes) fullRef else AndType(tp, fullRef) - selfTypeCache = selfInfo match { - case NoType => - fullRef - case tp: Type => - if (cls is Module) tp else withFullRef(tp) - case self: Symbol => - assert(!(cls is Module)) - withFullRef(self.info) + if (selfTypeCache == null) + selfTypeCache = { + def fullRef = fullyAppliedRef(cls.typeRef, cls.typeParams) + val given = givenSelfType + val raw = + if (!given.exists) fullRef + else if (cls is Module) given + else if (ctx.erasedTypes) fullRef + else AndType(given, fullRef) + raw//.asSeenFrom(prefix, cls.owner) } - } selfTypeCache } + + /** The explicitly given self type (self types of modules are assumed to be + * explcitly given here). + */ + override def givenSelfType(implicit ctx: Context): Type = selfInfo match { + case tp: Type => tp + case self: Symbol => self.info + } private var selfTypeCache: Type = null From 8e6d2c3d29bbb2a55fbc5da701c9951e839112cf Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 12 Apr 2015 18:05:11 +0200 Subject: [PATCH 4/9] Self type inhertance check Check that the self type of a class conforms to the self types of its parent classes. --- src/dotty/tools/dotc/typer/RefChecks.scala | 12 ++++++++++ test/dotc/tests.scala | 1 + tests/neg/selfInheritance.scala | 28 ++++++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 tests/neg/selfInheritance.scala diff --git a/src/dotty/tools/dotc/typer/RefChecks.scala b/src/dotty/tools/dotc/typer/RefChecks.scala index 9b14fffc07d2..6a1f3652b444 100644 --- a/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/src/dotty/tools/dotc/typer/RefChecks.scala @@ -71,6 +71,17 @@ object RefChecks { } } + /** Check that self type of this class conforms to self types of parents */ + private def checkSelfType(clazz: Symbol)(implicit ctx: Context): Unit = clazz.info match { + case cinfo: ClassInfo => + for (parent <- cinfo.classParents) { + val pself = parent.givenSelfType.asSeenFrom(clazz.thisType, parent.classSymbol) + if (pself.exists && !(cinfo.selfType <:< pself)) + ctx.error(d"illegal inheritance: self type ${cinfo.selfType} of $clazz does not conform to self type $pself of parent ${parent.classSymbol}", clazz.pos) + } + case _ => + } + // Override checking ------------------------------------------------------------ /** 1. Check all members of class `clazz` for overriding conditions. @@ -770,6 +781,7 @@ class RefChecks extends MiniPhase with SymTransformer { thisTransformer => override def transformTemplate(tree: Template)(implicit ctx: Context, info: TransformerInfo) = { val cls = ctx.owner checkOverloadedRestrictions(cls) + checkSelfType(cls) checkAllOverrides(cls) checkAnyValSubclass(cls) tree diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index a015b9efec1b..c3f501a52cd6 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -126,6 +126,7 @@ class tests extends CompilerTest { @Test def neg_i0281 = compileFile(negDir, "i0281-null-primitive-conforms", xerrors = 3) @Test def neg_moduleSubtyping = compileFile(negDir, "moduleSubtyping", xerrors = 4) @Test def neg_escapingRefs = compileFile(negDir, "escapingRefs", xerrors = 2) + @Test def neg_selfInheritance = compileFile(negDir, "selfInheritance", xerrors = 5) @Test def dotc = compileDir(dotcDir + "tools/dotc", failedOther)(allowDeepSubtypes ++ twice) // see dotc_core @Test def dotc_ast = compileDir(dotcDir + "tools/dotc/ast", failedOther ++ twice) diff --git a/tests/neg/selfInheritance.scala b/tests/neg/selfInheritance.scala new file mode 100644 index 000000000000..5f61c5bbbdb8 --- /dev/null +++ b/tests/neg/selfInheritance.scala @@ -0,0 +1,28 @@ +trait T { self: B => } + +abstract class A { self: B => + +} + +class B extends A with T { +} + +class C { self: B => + +} + +class D extends A // error + +class E extends T // error + +object Test { + + new B() {} + + new A() {} // error + + object O extends A // error + + object M extends C // error + +} From c37b408bcb47f494adfac5c94921df49ff3487c8 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 12 Apr 2015 18:06:37 +0200 Subject: [PATCH 5/9] New utility method: hasAttachment Used to test whether a tree has an attachment of a given key value. --- src/dotty/tools/dotc/util/Attachment.scala | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/util/Attachment.scala b/src/dotty/tools/dotc/util/Attachment.scala index e29bf13fe3e2..ed271e02c3da 100644 --- a/src/dotty/tools/dotc/util/Attachment.scala +++ b/src/dotty/tools/dotc/util/Attachment.scala @@ -19,7 +19,15 @@ object Attachment { val nx = next if (nx == null) None else if (nx.key eq key) Some(nx.value.asInstanceOf[V]) - else nx.getAttachment[V](key) + else nx.getAttachment(key) + } + + /** Does tree have an attachment corresponding to `key`? */ + final def hasAttachment[V](key: Key[V]): Boolean = { + val nx = next + if (nx == null) false + else if (nx.key eq key) true + else nx.hasAttachment(key) } /** The attachment corresponding to `key`. From 71a5179ef9d23b0b93d887a6d296c8a62b27672d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 12 Apr 2015 18:12:26 +0200 Subject: [PATCH 6/9] New attachments: ParentNew and AnnotNew The attachments label `New` nodes that are parents or annotations. These nodes will be treated specially when it comes to checking whether a class can be instantiated. Specifically, parents and Java-defined annotations are allowed to be abstract. Also: New convenience method: isAnnotation --- src/dotty/tools/dotc/ast/TreeInfo.scala | 6 ++++ .../tools/dotc/core/SymDenotations.scala | 7 +++++ src/dotty/tools/dotc/typer/Applications.scala | 28 +++++++++++++++++++ src/dotty/tools/dotc/typer/Namer.scala | 14 +++++++++- src/dotty/tools/dotc/typer/Typer.scala | 22 +++++++++++---- 5 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/dotty/tools/dotc/ast/TreeInfo.scala b/src/dotty/tools/dotc/ast/TreeInfo.scala index a7f89337c4b9..bb6e3eff3227 100644 --- a/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -82,6 +82,12 @@ trait TreeInfo[T >: Untyped <: Type] { self: Trees.Instance[T] => case Block(stats, expr) => methPart(expr) case mp => mp } + + /** If tree is an instance creation expression, its New node, otherwise tree itself */ + def newPart(tree: Tree) = methPart(tree) match { + case Select(nu: untpd.New, nme.CONSTRUCTOR) => nu + case _ => tree + } /** If tree is a closure, it's body, otherwise tree itself */ def closureBody(tree: tpd.Tree)(implicit ctx: Context): tpd.Tree = tree match { diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala index 3566595f2360..49b56e7d42cb 100644 --- a/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/src/dotty/tools/dotc/core/SymDenotations.scala @@ -382,6 +382,10 @@ object SymDenotations { /** is this symbol a trait representing a type lambda? */ final def isLambdaTrait(implicit ctx: Context): Boolean = isClass && name.startsWith(tpnme.LambdaPrefix) + + /** Is this symbol an annotation? */ + final def isAnnotation(implicit ctx: Context): Boolean = + derivesFrom(defn.AnnotationClass) /** Is this symbol a package object or its module class? */ def isPackageObject(implicit ctx: Context): Boolean = { @@ -986,6 +990,9 @@ object SymDenotations { def nonMemberTermRef(implicit ctx: Context): TermRef = TermRef.withFixedSym(owner.thisType, name.asTermName, symbol.asTerm) + + def typeRefWithArgs(implicit ctx: Context): Type = + typeRef.appliedTo(typeParams.map(_.typeRef)) /** The variance of this type parameter or type member as an Int, with * +1 = Covariant, -1 = Contravariant, 0 = Nonvariant, or not a type parameter diff --git a/src/dotty/tools/dotc/typer/Applications.scala b/src/dotty/tools/dotc/typer/Applications.scala index c5bd70c1ed71..5132e25dd990 100644 --- a/src/dotty/tools/dotc/typer/Applications.scala +++ b/src/dotty/tools/dotc/typer/Applications.scala @@ -525,6 +525,7 @@ trait Applications extends Compatibility { self: Typer => methPart(fun1).tpe match { case funRef: TermRef => + labelAnnotArgs(funRef.symbol, tree) tryEither { implicit ctx => val app = if (proto.argsAreTyped) new ApplyToTyped(tree, fun1, funRef, proto.typedArgs, pt) @@ -578,6 +579,33 @@ trait Applications extends Compatibility { self: Typer => else realApply } + /** If `meth` is a constructor of a Java annotation used as an annotation + * (as opposed to a normal `new`), label every `new` everywhere in its arguments + * (including nested occurrences) as annotations. + * See pos/java-interop/t294 for examples. + * This labeling is safe because Java annotations only accept constants and + * other annotations as arguments, so a `new` must be an allocation site for + * an annotation. + * The labeling is necessary because we need to avoid flagging `new`'s of Java + * annotation interfaces as errors because the interface is abstract. + */ + private def labelAnnotArgs(meth: Symbol, app: untpd.Apply)(implicit ctx: Context): Unit = { + import untpd._ + val isJavaAnnot = meth.isConstructor && meth.is(JavaDefined) && meth.owner.isAnnotation + if (newPart(app).hasAttachment(AnnotNew) && isJavaAnnot) { + val labelNewsAsAnnots = new TreeTraverser { + def traverse(tree: untpd.Tree)(implicit ctx: Context): Unit = { + tree match { + case tree: New => tree.putAttachment(AnnotNew, ()) + case _ => + } + traverseChildren(tree) + } + } + app.args.foreach(labelNewsAsAnnots.traverse) + } + } + /** Overridden in ReTyper to handle primitive operations that can be generated after erasure */ protected def handleUnexpectedFunType(tree: untpd.Apply, fun: Tree)(implicit ctx: Context): Tree = throw new Error(s"unexpected type.\n fun = $fun,\n methPart(fun) = ${methPart(fun)},\n methPart(fun).tpe = ${methPart(fun).tpe},\n tpe = ${fun.tpe}") diff --git a/src/dotty/tools/dotc/typer/Namer.scala b/src/dotty/tools/dotc/typer/Namer.scala index 10667f884928..7aed4c3cfd35 100644 --- a/src/dotty/tools/dotc/typer/Namer.scala +++ b/src/dotty/tools/dotc/typer/Namer.scala @@ -155,10 +155,21 @@ class Namer { typer: Typer => import untpd._ + /** Untyped tree was already typed, the attachment value is its typed versuon */ val TypedAhead = new Attachment.Key[tpd.Tree] + + /** Tree was expanded to by desugaring; the attachment value is its expansion */ val ExpandedTree = new Attachment.Key[Tree] + + /** Tree has been assigned a symbol, which is the attachment value */ val SymOfTree = new Attachment.Key[Symbol] + /** `New` node is part of a parent constructor of a class, and can therefore be abstract */ + val ParentNew = new Attachment.Key[Unit] + + /** `New` node is an annotation, can be abstract if the annotation is Java defined */ + val AnnotNew = new Attachment.Key[Unit] + /** A partial map from unexpanded member and pattern defs and to their expansions. * Populated during enterSyms, emptied during typer. */ @@ -528,7 +539,8 @@ class Namer { typer: Typer => case TypeApply(core, targs) => (core, targs) case core => (core, Nil) } - val Select(New(tpt), nme.CONSTRUCTOR) = core + val Select(nu @ New(tpt), nme.CONSTRUCTOR) = core + nu.putAttachment(ParentNew, ()) val targs1 = targs map (typedAheadType(_)) val ptype = typedAheadType(tpt).tpe appliedTo targs1.tpes if (ptype.typeParams.isEmpty) ptype diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index cf0762ef01dd..414feb56b197 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -25,7 +25,7 @@ import EtaExpansion.etaExpand import dotty.tools.dotc.transform.Erasure.Boxing import util.Positions._ import util.common._ -import util.SourcePosition +import util.{SourcePosition, Attachment} import collection.mutable import annotation.tailrec import Implicits._ @@ -868,7 +868,8 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit } def typedAnnotation(annot: untpd.Tree)(implicit ctx: Context): Tree = track("typedAnnotation") { - typed(annot, defn.AnnotationClass.typeRef) + labelNew(annot, AnnotNew) + typedExpr(annot, defn.AnnotationClass.typeRef) } def typedValDef(vdef: untpd.ValDef, sym: Symbol)(implicit ctx: Context) = track("typedValDef") { @@ -940,11 +941,20 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit */ def ensureConstrCall(cls: ClassSymbol, parents: List[Tree])(implicit ctx: Context): List[Tree] = { val firstParent :: otherParents = parents - if (firstParent.isType && !(cls is Trait)) - typed(untpd.New(untpd.TypedSplice(firstParent), Nil)) :: otherParents - else parents + if (firstParent.isType && !(cls is Trait)) { + val constr = untpd.New(untpd.TypedSplice(firstParent), Nil) + labelNew(constr, ParentNew) + typed(constr) :: otherParents + } else parents } + /** If `tree` is an instance creation expression, attach the given label to its `New` node */ + def labelNew(tree: untpd.Tree, label: Attachment.Key[Unit]): Unit = + untpd.newPart(tree) match { + case nu: untpd.New => nu.putAttachment(label, ()) + case _ => + } + /** Overridden in retyper */ def checkVariance(tree: Tree)(implicit ctx: Context) = VarianceChecker.check(tree) @@ -971,7 +981,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit } def typedAnnotated(tree: untpd.Annotated, pt: Type)(implicit ctx: Context): Tree = track("typedAnnotated") { - val annot1 = typedExpr(tree.annot, defn.AnnotationClass.typeRef) + val annot1 = typedAnnotation(tree.annot) val arg1 = typed(tree.arg, pt) if (ctx.mode is Mode.Type) assignType(cpy.Annotated(tree)(annot1, arg1), annot1, arg1) From a9a92f2fce255b2f571e59ee071a00b5b063c070 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 12 Apr 2015 18:20:24 +0200 Subject: [PATCH 7/9] Check `New` nodes for instantiability. - Abstract classes cannot be instantiated (exceptions: parent news and Java annotations) - Instantiateed class must conform to its self type. --- src/dotty/tools/dotc/typer/Checking.scala | 15 ++++++--- src/dotty/tools/dotc/typer/Typer.scala | 7 ++++- test/dotc/tests.scala | 1 + tests/neg/instantiateAbstract.scala | 38 +++++++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 tests/neg/instantiateAbstract.scala diff --git a/src/dotty/tools/dotc/typer/Checking.scala b/src/dotty/tools/dotc/typer/Checking.scala index a547b3f178f1..f7703ce80dd0 100644 --- a/src/dotty/tools/dotc/typer/Checking.scala +++ b/src/dotty/tools/dotc/typer/Checking.scala @@ -243,18 +243,25 @@ trait Checking { /** Check that `tp` is a class type with a stable prefix. Also: * If `traitReq` is true, check that `tp` refers to a trait. - * If `concreteReq` is true, check that `tp` refers to a nonAbstract class. + * If `concreteReq` is true, check that `tp` refers to a nonAbstract class + * and that the instance conforms to the self type of the created class. * Stability checking is disabled in phases after RefChecks. * @return `tp` itself if it is a class or trait ref, ObjectClass.typeRef if not. */ def checkClassTypeWithStablePrefix(tp: Type, pos: Position, traitReq: Boolean = false, concreteReq: Boolean = false)(implicit ctx: Context): Type = tp.underlyingClassRef(refinementOK = false) match { case tref: TypeRef => + val cls = tref.symbol if (ctx.phase <= ctx.refchecksPhase) checkStable(tref.prefix, pos) - if (traitReq && !(tref.symbol is Trait)) + if (traitReq && !(cls is Trait)) ctx.error(d"$tref is not a trait", pos) - if (concreteReq && false && (tref.symbol is AbstractOrTrait)) - ctx.error(d"${tref.symbol} is abstract; cannot be instantiated, owner = ${ctx.owner}", pos) + if (concreteReq) { + if (cls.is(AbstractOrTrait)) + ctx.error(d"$cls is abstract; cannot be instantiated", pos) + val selfType = tp.givenSelfType.asSeenFrom(tref.prefix, cls.owner) + if (!cls.is(Module) && selfType.exists && !(tp <:< selfType)) + ctx.error(d"$tp does not conform to its self type $selfType; cannot be instantiated") + } tp case _ => ctx.error(d"$tp is not a class type", pos) diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index 414feb56b197..c8321279e146 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -348,7 +348,12 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit typed(cpy.Block(tree)(clsDef :: Nil, New(Ident(x), Nil)), pt) case _ => val tpt1 = typedType(tree.tpt) - checkClassTypeWithStablePrefix(tpt1.tpe, tpt1.pos, concreteReq = !ctx.owner.isClass) + val cls = tpt1.tpe.classSymbol + val isJavaAnnot = + tree.hasAttachment(AnnotNew) && cls.is(JavaDefined) && cls.isAnnotation + val canBeAbstract = + tree.hasAttachment(ParentNew) || isJavaAnnot || ctx.isAfterTyper + checkClassTypeWithStablePrefix(tpt1.tpe, tree.pos, concreteReq = !canBeAbstract) assignType(cpy.New(tree)(tpt1), tpt1) // todo in a later phase: checkInstantiatable(cls, tpt1.pos) } diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index c3f501a52cd6..b9ed6fb0311e 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -126,6 +126,7 @@ class tests extends CompilerTest { @Test def neg_i0281 = compileFile(negDir, "i0281-null-primitive-conforms", xerrors = 3) @Test def neg_moduleSubtyping = compileFile(negDir, "moduleSubtyping", xerrors = 4) @Test def neg_escapingRefs = compileFile(negDir, "escapingRefs", xerrors = 2) + @Test def neg_instantiateAbstract = compileFile(negDir, "instantiateAbstract", xerrors = 8) @Test def neg_selfInheritance = compileFile(negDir, "selfInheritance", xerrors = 5) @Test def dotc = compileDir(dotcDir + "tools/dotc", failedOther)(allowDeepSubtypes ++ twice) // see dotc_core diff --git a/tests/neg/instantiateAbstract.scala b/tests/neg/instantiateAbstract.scala new file mode 100644 index 000000000000..1737f0a970cb --- /dev/null +++ b/tests/neg/instantiateAbstract.scala @@ -0,0 +1,38 @@ +abstract class AA + +trait TT + +class A { self: B => + +} + +@scala.annotation.Annotation class C // error + +class B extends A() { +} + +object Test { + + @scala.annotation.Annotation type T = String // ok, annotations do not count as new + @scala.annotation.Annotation val x = 1 // ok + @scala.annotation.Annotation def f = 1 //ok + + (1: @scala.annotation.Annotation) // ok + + + new AA // error + + new TT // error + + new A // error + +// the following are OK in Typer but would be caught later in RefChecks + + new A() {} + + new AA() {} + + object O extends A + + object OO extends AA +} From bb48acb607b8c30a16cc45bb9e05470d80b0b04f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 12 Apr 2015 18:24:26 +0200 Subject: [PATCH 8/9] Print supressed error message if it comes first. An error message might be suppressed because it contains internal code. But if it is the first one, we should still print it. Otherwise, we risk a scenario where we see "2 errors" but none is printed. --- src/dotty/tools/dotc/reporting/ConsoleReporter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/reporting/ConsoleReporter.scala b/src/dotty/tools/dotc/reporting/ConsoleReporter.scala index a7f7f70bb21a..f07f43a63675 100644 --- a/src/dotty/tools/dotc/reporting/ConsoleReporter.scala +++ b/src/dotty/tools/dotc/reporting/ConsoleReporter.scala @@ -41,7 +41,7 @@ class ConsoleReporter( } override def doReport(d: Diagnostic)(implicit ctx: Context): Unit = - if (!d.isSuppressed) d match { + if (!d.isSuppressed || !hasErrors) d match { case d: Error => printMessageAndPos(s"error: ${d.msg}", d.pos) if (ctx.settings.prompt.value) displayPrompt() From 60828d2d932476b3fa3dfa254da398efb9abdccd Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 12 Apr 2015 18:28:31 +0200 Subject: [PATCH 9/9] Tweaks relative to error reporting --- src/dotty/tools/dotc/ast/Desugar.scala | 2 +- src/dotty/tools/dotc/typer/Checking.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/ast/Desugar.scala b/src/dotty/tools/dotc/ast/Desugar.scala index 165f4f53548d..0c13d1ecc2d4 100644 --- a/src/dotty/tools/dotc/ast/Desugar.scala +++ b/src/dotty/tools/dotc/ast/Desugar.scala @@ -786,7 +786,7 @@ object desugar { New(ref(defn.RepeatedAnnot.typeRef), Nil :: Nil), AppliedTypeTree(ref(seqClass.typeRef), t)) } else { - assert(ctx.mode.isExpr, ctx.mode) + assert(ctx.mode.isExpr || ctx.reporter.hasErrors, ctx.mode) Select(t, op) } case PrefixOp(op, t) => diff --git a/src/dotty/tools/dotc/typer/Checking.scala b/src/dotty/tools/dotc/typer/Checking.scala index f7703ce80dd0..6b6a59c98f40 100644 --- a/src/dotty/tools/dotc/typer/Checking.scala +++ b/src/dotty/tools/dotc/typer/Checking.scala @@ -232,7 +232,7 @@ trait Checking { /** Check that type `tp` is stable. */ def checkStable(tp: Type, pos: Position)(implicit ctx: Context): Unit = - if (!tp.isStable) + if (!tp.isStable && !tp.isErroneous) ctx.error(d"$tp is not stable", pos) /** Check that type `tp` is a legal prefix for '#'.