diff options
author | Paul Phillips <paulp@improving.org> | 2010-07-03 00:56:00 +0000 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2010-07-03 00:56:00 +0000 |
commit | 39e4641ec932965eaa9ef30b3954aef3f0579fa5 (patch) | |
tree | 6dce13c4da159d528826807c5cee1ff7b844fd3d | |
parent | 322e856f132c4ee8ac651848a041b693f830371b (diff) | |
download | scala-39e4641ec932965eaa9ef30b3954aef3f0579fa5.tar.gz scala-39e4641ec932965eaa9ef30b3954aef3f0579fa5.tar.bz2 scala-39e4641ec932965eaa9ef30b3954aef3f0579fa5.zip |
When compilation fails because of an unimplemen...
When compilation fails because of an unimplemented abstract var, give
a more precise error message about what happened. Also avoid issuing
the same error twice because neither getter nor setter is implemented.
Closes #36, review by rytz.
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 64 | ||||
-rwxr-xr-x | src/library/scala/reflect/generic/Symbols.scala | 1 | ||||
-rw-r--r-- | test/files/neg/abstract-vars.check | 21 | ||||
-rw-r--r-- | test/files/neg/abstract-vars.scala | 29 |
4 files changed, 96 insertions, 19 deletions
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 87a11497fd..479b843931 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -399,9 +399,8 @@ abstract class RefChecks extends InfoTransform { } printMixinOverrideErrors() - // 2. Check that only abstract classes have deferred members - if (clazz.isClass && !clazz.isTrait) { - def isClazzAbstract = clazz hasFlag ABSTRACT + // Verifying a concrete class has nothing unimplemented. + if (clazz.isClass && !clazz.isTrait && !(clazz hasFlag ABSTRACT)) { val abstractErrors = new ListBuffer[String] def abstractErrorMessage = // a little formatting polish @@ -421,8 +420,7 @@ abstract class RefChecks extends InfoTransform { def javaErasedOverridingSym(sym: Symbol): Symbol = clazz.tpe.nonPrivateMemberAdmitting(sym.name, BRIDGE).filter(other => - !other.isDeferred && - (other hasFlag JAVA) && { + !other.isDeferred && other.isJavaDefined && { val tp1 = erasure.erasure(clazz.thisType.memberType(sym)) val tp2 = erasure.erasure(clazz.thisType.memberType(other)) atPhase(currentRun.erasurePhase.next)(tp1 matches tp2) @@ -430,20 +428,46 @@ abstract class RefChecks extends InfoTransform { def ignoreDeferred(member: Symbol) = isAbstractTypeWithoutFBound(member) || - ((member hasFlag JAVA) && javaErasedOverridingSym(member) != NoSymbol) - - for (member <- clazz.tpe.nonPrivateMembersAdmitting(VBRIDGE)) - if (member.isDeferred && !isClazzAbstract && !ignoreDeferred(member)) { - abstractClassError( - false, infoString(member) + " is not defined" + analyzer.varNotice(member)) - } else if ((member hasFlag ABSOVERRIDE) && member.isIncompleteIn(clazz)) { - val other = member.superSymbol(clazz); - abstractClassError(true, - infoString(member) + " is marked `abstract' and `override'" + - (if (other != NoSymbol) - " and overrides incomplete superclass member " + infoString(other) - else ", but no concrete implementation could be found in a base class")) + (member.isJavaDefined && javaErasedOverridingSym(member) != NoSymbol) + + // 2. Check that only abstract classes have deferred members + def checkNoAbstractMembers() = { + // Avoid spurious duplicates: first gather any missing members. + def memberList = clazz.tpe.nonPrivateMembersAdmitting(VBRIDGE) + val (missing, rest) = memberList partition (m => m.isDeferred && !ignoreDeferred(m)) + // Group missing members by the underlying symbol. + val grouped = missing groupBy (analyzer underlying _ name) + + for (member <- missing) { + def undefined(msg: String) = abstractClassError(false, infoString(member) + " is not defined" + msg) + val underlying = analyzer.underlying(member) + + // Give a specific error message for abstract vars based on why it fails: + // It could be unimplemented, have only one accessor, or be uninitialized. + if (underlying.isVariable) { + // If both getter and setter are missing, squelch the setter error. + val isMultiple = grouped(underlying.name).size > 1 + // TODO: messages shouldn't be spread over two files, and varNotice is not a clear name + if (member.isSetter && isMultiple) () + else undefined( + if (member.isSetter) "\n(Note that an abstract var requires a setter in addition to the getter)" + else if (member.isGetter && !isMultiple) "\n(Note that an abstract var requires a getter in addition to the setter)" + else analyzer.varNotice(member) + ) + } + else undefined("") + } + + // Check the remainder for invalid absoverride. + for (member <- rest ; if ((member hasFlag ABSOVERRIDE) && member.isIncompleteIn(clazz))) { + val other = member.superSymbol(clazz) + val explanation = + if (other != NoSymbol) " and overrides incomplete superclass member " + infoString(other) + else ", but no concrete implementation could be found in a base class" + + abstractClassError(true, infoString(member) + " is marked `abstract' and `override'" + explanation) } + } // 3. Check that concrete classes do not have deferred definitions // that are not implemented in a subclass. @@ -467,7 +491,9 @@ abstract class RefChecks extends InfoTransform { if (!parents.isEmpty && parents.head.typeSymbol.hasFlag(ABSTRACT)) checkNoAbstractDecls(parents.head.typeSymbol) } - if (abstractErrors.isEmpty && !isClazzAbstract) + + checkNoAbstractMembers() + if (abstractErrors.isEmpty) checkNoAbstractDecls(clazz) if (abstractErrors.nonEmpty) diff --git a/src/library/scala/reflect/generic/Symbols.scala b/src/library/scala/reflect/generic/Symbols.scala index 2f5e0624ab..f1226c7e19 100755 --- a/src/library/scala/reflect/generic/Symbols.scala +++ b/src/library/scala/reflect/generic/Symbols.scala @@ -119,6 +119,7 @@ trait Symbols { self: Universe => def isTrait: Boolean = isClass && hasFlag(TRAIT) // refined later for virtual classes. final def hasDefault = isParameter && hasFlag(DEFAULTPARAM) final def isAbstractClass = isClass && hasFlag(ABSTRACT) + // XXX This is unlikely to be correct: it's not looking for the ABSOVERRIDE flag? final def isAbstractOverride = isTerm && hasFlag(ABSTRACT) && hasFlag(OVERRIDE) final def isBridge = hasFlag(BRIDGE) final def isCase = hasFlag(CASE) diff --git a/test/files/neg/abstract-vars.check b/test/files/neg/abstract-vars.check new file mode 100644 index 0000000000..8aa47745f6 --- /dev/null +++ b/test/files/neg/abstract-vars.check @@ -0,0 +1,21 @@ +abstract-vars.scala:5: error: class Fail1 needs to be abstract, since variable x is not defined +(Note that variables need to be initialized to be defined) +class Fail1 extends A { + ^ +abstract-vars.scala:9: error: class Fail2 needs to be abstract, since variable x in class A of type Int is not defined +(Note that variables need to be initialized to be defined) +class Fail2 extends A { } + ^ +abstract-vars.scala:11: error: class Fail3 needs to be abstract, since variable x in class A of type Int is not defined +(Note that an abstract var requires a setter in addition to the getter) +class Fail3 extends A { + ^ +abstract-vars.scala:14: error: class Fail4 needs to be abstract, since variable x in class A of type Int is not defined +(Note that an abstract var requires a setter in addition to the getter) +class Fail4 extends A { + ^ +abstract-vars.scala:18: error: class Fail5 needs to be abstract, since variable x in class A of type Int is not defined +(Note that an abstract var requires a getter in addition to the setter) +class Fail5 extends A { + ^ +5 errors found diff --git a/test/files/neg/abstract-vars.scala b/test/files/neg/abstract-vars.scala new file mode 100644 index 0000000000..df6109d3a8 --- /dev/null +++ b/test/files/neg/abstract-vars.scala @@ -0,0 +1,29 @@ +abstract class A { + var x: Int +} + +class Fail1 extends A { + var x: Int +} + +class Fail2 extends A { } + +class Fail3 extends A { + val x: Int = 5 +} +class Fail4 extends A { + def x: Int = 5 +} + +class Fail5 extends A { + def x_=(y: Int) = () +} + +class Success1 extends A { + val x: Int = 5 + def x_=(y: Int) = () +} + +class Success2 extends A { + var x: Int = 5 +} |