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

Named arg may be deprecatedName #21588

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Sep 14, 2024

For a method definition

def f(@deprecatedName("x") x: Int, @deprecatedName y: Int, @deprecatedName("w") z: Int) = x+y+z

emit a deprecation warning if x, y, or w are named in an application.

Fixes #19077

Supersedes #19086

The check for a deprecated name is when an arg is deemed missing: that is when draining the list of args and the current arg is named. Instead of missingArgs = true, first check whether this arg is a deprecated name for the current parameter. Fix up that arg to use the canonical name and continue.

handlePositional also consumes leading named args that are in position. Common applications such as f(i, b = true, j) need not handleNamed.

Checking for a deprecated name is expensive but only happens for named args. (Looking for the deprecated name as a fallback would be fine, but every named arg is checked; that is not usually every arg.)

@som-snytt
Copy link
Contributor Author

som-snytt commented Sep 14, 2024

Taking the prefix of both named and positional args while no positions change results in positional after named argument becoming parameter is already instantiated in that subtle test 18122; that's when there is a repeating parameter.

Edit: that was the line of the test I was reading a year ago at #18363 (comment) If that is the only casualty and both messages are correct, then the change may be acceptable if not desirable.

@som-snytt
Copy link
Contributor Author

An interesting pc test failure involving overloading and polytypes.

@som-snytt
Copy link
Contributor Author

That test is still

image

@som-snytt
Copy link
Contributor Author

som-snytt commented Feb 17, 2025

That is

object X { def m[T](i: Int)(s: String) = s*i; def m[T](i: Int)(d: Double) = d*i }

object Y:
  def f = X.m(42)(s = "") // overloading resolution objects to methRef.symbol.paramSymss

where paramss will step through param lists while info.paramNames is nonempty, but param namess has the "extra" type param name, so params are exhausted and Nil.head.

SETRAW(method m) List(List(val d))
SETRAW(method m) List(List(val s))
reorder (List(s = "")) meth = TermRef(NoPrefix,method m) or method m
DRILL(method m) List(T) where pss List(List(val d))
DRILL(method m) List(d) where pss List()

Edit: during overload res, when considering the next param list, raw paramss are set to the value params, but the info is not stripped. Not sure the workaround belongs in paramSymss, so if reorder sees the special attachment, it relies on rawParamss instead. (This is for retrieving the deprecated name annotation.)

@som-snytt
Copy link
Contributor Author

som-snytt commented Feb 22, 2025

The last bit was to use rawParamss instead of failing paramSymss if it looks like we're in the middle of overload resolution.

I especially like the fringe improvement to i18122. I know stdlib has some deprecated param names; and it's possible that named args are underused in general; how many lives does the ability to migrate param names either improve or save outright?

@som-snytt som-snytt marked this pull request as ready for review February 22, 2025 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dotc ignores deprecatedName
1 participant