Merge pull request scala#5331 from adriaanm/pr5282-reword

SD-128 fix override checks for default methods

Author: adriaanm

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -419,7 +419,7 @@ abstract class RefChecks extends Transform {
             overrideError("cannot be used here - classes can only override abstract types")
           } else if (other.isEffectivelyFinal) { // (1.2)
             overrideError("cannot override final member")
-          } else if (!other.isDeferredOrJavaDefault && !other.hasFlag(JAVA_DEFAULTMETHOD) && !member.isAnyOverride && !member.isSynthetic) { // (*)
+          } else if (!other.isDeferred && !member.isAnyOverride && !member.isSynthetic) { // (*)
             // (*) Synthetic exclusion for (at least) default getters, fixes SI-5178. We cannot assign the OVERRIDE flag to
             // the default getter: one default getter might sometimes override, sometimes not. Example in comment on ticket.
               if (isNeitherInClass && !(other.owner isSubClass member.owner))
@@ -606,7 +606,7 @@ abstract class RefChecks extends Transform {
         def checkNoAbstractMembers(): Unit = {
           // Avoid spurious duplicates: first gather any missing members.
           def memberList = clazz.info.nonPrivateMembersAdmitting(VBRIDGE)
-          val (missing, rest) = memberList partition (m => m.isDeferredNotJavaDefault && !ignoreDeferred(m))
+          val (missing, rest) = memberList partition (m => m.isDeferred && !ignoreDeferred(m))
           // Group missing members by the name of the underlying symbol,
           // to consolidate getters and setters.
           val grouped = missing groupBy (sym => analyzer.underlyingSymbol(sym).name)

src/library/scala/collection/convert/Wrappers.scala

@@ -28,7 +28,7 @@ private[collection] trait Wrappers {
     def next() = underlying.next()
     def hasMoreElements = underlying.hasNext
     def nextElement() = underlying.next()
-    def remove() = throw new UnsupportedOperationException
+    override def remove() = throw new UnsupportedOperationException
   }
 
   class ToIteratorWrapper[A](underlying : Iterator[A]) {
@@ -113,7 +113,7 @@ private[collection] trait Wrappers {
       var prev: Option[A] = None
       def hasNext = ui.hasNext
       def next = { val e = ui.next(); prev = Some(e); e }
-      def remove = prev match {
+      override def remove() = prev match {
         case Some(e) =>
           underlying match {
             case ms: mutable.Set[a] =>
@@ -200,7 +200,7 @@ private[collection] trait Wrappers {
           }
         }
 
-        def remove() {
+        override def remove() {
           prev match {
             case Some(k) =>
               underlying match {
@@ -293,24 +293,24 @@ private[collection] trait Wrappers {
 
   class ConcurrentMapWrapper[A, B](override val underlying: concurrent.Map[A, B]) extends MutableMapWrapper[A, B](underlying) with juc.ConcurrentMap[A, B] {
 
-    def putIfAbsent(k: A, v: B) = underlying.putIfAbsent(k, v) match {
+    override def putIfAbsent(k: A, v: B) = underlying.putIfAbsent(k, v) match {
       case Some(v) => v
       case None => null.asInstanceOf[B]
     }
 
-    def remove(k: AnyRef, v: AnyRef) = try {
+    override def remove(k: AnyRef, v: AnyRef) = try {
       underlying.remove(k.asInstanceOf[A], v.asInstanceOf[B])
     } catch {
       case ex: ClassCastException =>
         false
     }
 
-    def replace(k: A, v: B): B = underlying.replace(k, v) match {
+    override def replace(k: A, v: B): B = underlying.replace(k, v) match {
       case Some(v) => v
       case None => null.asInstanceOf[B]
     }
 
-    def replace(k: A, oldval: B, newval: B) = underlying.replace(k, oldval, newval)
+    override def replace(k: A, oldval: B, newval: B) = underlying.replace(k, oldval, newval)
   }
 
   /** Wraps a concurrent Java map as a Scala one.  Single-element concurrent

src/reflect/scala/reflect/internal/Definitions.scala

@@ -855,7 +855,7 @@ trait Definitions extends api.StandardDefinitions {
         // must filter out "universal" members (getClass is deferred for some reason)
         val deferredMembers = (
           tp.membersBasedOnFlags(excludedFlags = BridgeAndPrivateFlags, requiredFlags = METHOD).toList.filter(
-            mem => mem.isDeferredNotJavaDefault && !isUniversalMember(mem)
+            mem => mem.isDeferred && !isUniversalMember(mem)
           ) // TODO: test
         )
 

src/reflect/scala/reflect/internal/HasFlags.scala

@@ -122,9 +122,6 @@ trait HasFlags {
   def isTrait               = hasFlag(TRAIT) && !hasFlag(PARAM)
   def isTraitOrInterface    = isTrait || isInterface
 
-  def isDeferredOrJavaDefault  = hasFlag(DEFERRED | JAVA_DEFAULTMETHOD)
-  def isDeferredNotJavaDefault = isDeferred && !hasFlag(JAVA_DEFAULTMETHOD)
-
   def flagBitsToString(bits: Long): String = {
     // Fast path for common case
     if (bits == 0L) "" else {

test/files/neg/sd128.check

@@ -0,0 +1,17 @@
+Test.scala:4: error: class C1 inherits conflicting members:
+  method f in trait A of type ()Int  and
+  method f in trait T of type => Int
+(Note: this can be resolved by declaring an override in class C1.)
+class C1 extends A with T // error
+      ^
+Test.scala:5: error: class C2 inherits conflicting members:
+  method f in trait T of type => Int  and
+  method f in trait A of type ()Int
+(Note: this can be resolved by declaring an override in class C2.)
+class C2 extends T with A // error
+      ^
+Test.scala:14: error: overriding method f in trait A of type ()Int;
+ method f needs `override' modifier
+  def f() = 9999 // need override modifier
+      ^
+three errors found

test/files/neg/sd128/A.java

@@ -0,0 +1,3 @@
+interface A {
+  default int f() { return -10; }
+}

test/files/neg/sd128/Test.scala

@@ -0,0 +1,19 @@
+trait T {
+  def f = 99
+}
+class C1 extends A with T // error
+class C2 extends T with A // error
+
+trait U extends A {
+  override def f = 999
+}
+class D1 extends A with U // OK
+class D2 extends U with A // OK
+
+class E1 extends A {
+  def f() = 9999 // need override modifier
+}
+
+class E2 extends A {
+  override def f() = 9999 // OK
+}