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

Default arguments are no longer available for overriden method #16006

Closed
WojciechMazur opened this issue Sep 9, 2022 · 1 comment · Fixed by #16009
Closed

Default arguments are no longer available for overriden method #16006

WojciechMazur opened this issue Sep 9, 2022 · 1 comment · Fixed by #16009
Assignees
Labels
itype:bug regression This worked in a previous version but doesn't anymore
Milestone

Comments

@WojciechMazur
Copy link
Contributor

WojciechMazur commented Sep 9, 2022

Regression found in the Open CB #5274 for scalanlp/breeze

Bisect points to d2eeef1

Compiler version

Scala 3.2.2-RC1-bin-20220907-a503b7a-NIGHTLY

Minimized code

class X: 
  def toString(maxLines: Int = 10, maxWidth: Int = 10): String = (maxLines -> maxWidth).toString

class Foo extends X:
  override def toString(maxLines: Int, maxWidth: Int): String = s"overriden ($maxLines, $maxWidth)"
  override def toString(): String = toString(maxLines = 3)


@main def Test = {
  println(Foo().toString())
}

Output

In Scala 3.2.1-RC1

Compiled project (Scala 3.2.1-RC1, JVM)
overriden (3, 10)

In nightly

Compiling project (Scala 3.2.2-RC1-bin-20220907-a503b7a-NIGHTLY, JVM)
[error] ./test.scala:7:37: None of the overloaded alternatives of method toString in class Foo with types
[error]  (): String
[error]  (maxLines: Int, maxWidth: Int): String
[error] match arguments ((3 : Int))
[error]   override def toString(): String = toString(maxLines = 3)
[error]   

Expectation

Should be decided if behaviour follows the specification and fixed.

@WojciechMazur WojciechMazur added itype:bug stat:needs spec regression This worked in a previous version but doesn't anymore stat:needs triage Every issue needs to have an "area" and "itype" label labels Sep 9, 2022
@odersky
Copy link
Contributor

odersky commented Sep 9, 2022

That's not intentional. It's probably caused by #15962. I am on it.

@odersky odersky self-assigned this Sep 9, 2022
@odersky odersky removed stat:needs spec stat:needs triage Every issue needs to have an "area" and "itype" label labels Sep 9, 2022
odersky added a commit to dotty-staging/dotty that referenced this issue Sep 9, 2022
Fixes scala#16006

scala#16006 started failing since we now count the number of default parameters
for better precision in overloading resolution. But that count was wrong since
it did not take inherited default getters from overridden methods into account.
We fix this by setting the HasDefault flag also for parameters that don't have
a default value themselves, but that correspond to parameters with default
values in overridden methods.
odersky added a commit that referenced this issue Oct 9, 2022
…getters (#16009)

Fixes #16006
Fixes #15287

When resolving overloading using parameter lists after the first one, we
used mapped
symbols that forgot about the prefix of the original call and how many
parameters were
skipped. Consequently, overloading resolution got confused when there
were default
parameters in following parameter lists. We now keep track of these
values in an
annotation that gets added to the mapped symbols.

We also use `findDefaultGetter` directly to compute the number of
default parameters
in `sizeFits`. The previous scheme of checking the `HasParam` flag of
parameters
fails for default values inherited from overriden methods.
@Kordyjan Kordyjan added this to the 3.2.2 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
itype:bug regression This worked in a previous version but doesn't anymore
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants