Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression in overriding methods with default arguments #5178

Closed
scabug opened this Issue Nov 10, 2011 · 8 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

scabug commented Nov 10, 2011

This compiles under 2.9.1, but under trunk:

abstract class FileOps {
  def withLock[R](start: Long = 0): Option[R]
}

trait DefaultFileOps {
  self: DefaultPath =>
  
  override def withLock[R](start: Long = 5): Option[R] = None
}

class DefaultPath extends FileOps with DefaultFileOps { }
bip.scala:11: error: overriding method withLock$default$1 in class FileOps of type [R]=> Long;
 method withLock$default$1 in trait DefaultFileOps of type [R]=> Long needs `override' modifier
class DefaultPath extends FileOps with DefaultFileOps { }
      ^
one error found
@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 10, 2011

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 10, 2011

@paulp said:
Caused by me in r25709.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 10, 2011

Commit Message Bot (anonymous) said:
(extempore in r25979) Fix for regression in overriding with defaults.

I should know better than to behave as if usable inferences can
be drawn from a comment like "SYNTHETIC because of DEVIRTUALIZE".
Maybe it was even true when it was written, but no more.
Closes #5178, no review.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 11, 2011

@lrytz said:
i think we should rather put the correct flags instead of having a special case in override checking.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 11, 2011

@paulp said:
That would be great! I was just putting it back the way it was before I regressed it, because this was preventing me from building scala-io. However I don't know what the correct flags are. (The default getters should inherit more flags from the relevant methods?)

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 11, 2011

@lrytz said:
no, but if you override a default, a second default getter is generated which overrides the one in the superclass. it should have the override flag. i'll have a look at some point.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Nov 11, 2011

@paulp said:
I think it has the right flags in the straightforward case; it's when the self-type is involved that it doesn't. (See test/files/pos/t5178.scala.)

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 9, 2012

@lrytz said:
we cannot assign the correct OVERRIDE flags to default getters. one and the same default getter might sometimes override another one, sometimes not, example below.

this is a consequence of the fact that we allow overriding defaults without override modifiers.

so the fix in 9f9932bd20 is left in place, a comment is added in 5d90d00108

trait A1 { def f(x: Int): Int }
trait A2 extends A1 { def f(x: Int = 1): Int }

trait B { self: A1 =>
  def f(x: Int = 2): Int = x
}

object t {
  new A1 with B // here, B.f$default$1 should not have an 'override' flag
  new A2 with B // here, B.f$default$1 would need an 'override' flag
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.