Skip to content

Inferred Contextual params are less nameable#24381

Merged
som-snytt merged 2 commits intoscala:mainfrom
som-snytt:issue/24375-context-function-param-names
Dec 8, 2025
Merged

Inferred Contextual params are less nameable#24381
som-snytt merged 2 commits intoscala:mainfrom
som-snytt:issue/24375-context-function-param-names

Conversation

@som-snytt
Copy link
Copy Markdown
Contributor

@som-snytt som-snytt commented Nov 12, 2025

Fixes #24375

The original name can be recovered for messages.

odersky
odersky reviewed Nov 12, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@odersky odersky left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach looks workable to me. The test failures seem to come from this part in Message.scala:

    override def ParamRefNameString(param: ParamRef): String =
      seen.record(super.ParamRefNameString(param), param.isInstanceOf[TypeParamRef], param)

Maybe we can try to just turn off seen.record here, i.e. drop the whole def? I am not sure more explanations of parameter names help. But one has to try it out and look at the changes.

@som-snytt som-snytt force-pushed the issue/24375-context-function-param-names branch from 73533af to dab9edb Compare November 12, 2025 10:36
@som-snytt
Copy link
Copy Markdown
Contributor Author

som-snytt commented Nov 12, 2025

Just a stop-gap update. I will revisit the parameter disambiguation.

Now it is aware that the written l and the inferred lcontextual$1 are not the same. Or maybe just let it ignore the difference.

def withFile[T](op: (l: caps.Capability) ?-> (f: File^{l}) => T): T =
  op(using caps.cap)(new File)

def test =
  withFile: f =>       // error with level checking, was OK under both schemes before
    val o = Service()
    o.file = f
    o.log

@odersky
Copy link
Copy Markdown
Contributor

odersky commented Nov 12, 2025

I think @Sporarum's comment on the issue makes sense.

odersky
odersky reviewed Nov 12, 2025
View reviewed changes
Comment thread compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala Outdated
protected def ParamRefNameString(param: ParamRef): String =
ParamRefNameString(param.binder.paramNames(param.paramNum))
val name = param.binder.paramNames(param.paramNum)
ParamRefNameString:
Copy link
Copy Markdown
Contributor

@odersky odersky Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answering @Sporarum's comment on the issue :
#24375 (comment)

Maybe we should work on the toString function for ContectFunctionParamNames instead of intervening here.

@som-snytt som-snytt force-pushed the issue/24375-context-function-param-names branch from e0a5939 to cbc28fc Compare November 16, 2025 08:15
@som-snytt
Copy link
Copy Markdown
Contributor Author

som-snytt commented Nov 16, 2025

The captures tests start to fail in the presence of java.lang.IO because of the constructor proxy shadowing rule.

There are other failing tests on recent JDKs because of the way lambdas are printed, etc.

@som-snytt som-snytt force-pushed the issue/24375-context-function-param-names branch from cbc28fc to 88af888 Compare November 16, 2025 17:07
@som-snytt som-snytt marked this pull request as ready for review November 17, 2025 20:31
@som-snytt
Copy link
Copy Markdown
Contributor Author

som-snytt commented Nov 17, 2025

Pushed the same "fix" downstack, but I haven't given any thought to what is desirable in a superscript, or whether other kinds would benefit from similar tweaking.

@Gedochao Gedochao requested a review from noti0na1 December 3, 2025 09:28
noti0na1
noti0na1 approved these changes Dec 8, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@noti0na1 noti0na1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@som-snytt som-snytt merged commit 4247597 into scala:main Dec 8, 2025
45 checks passed
@som-snytt som-snytt deleted the issue/24375-context-function-param-names branch December 8, 2025 16:33
@WojciechMazur WojciechMazur added this to the 3.8.1 milestone Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@odersky odersky odersky left review comments

@noti0na1 noti0na1 noti0na1 approved these changes

Assignees

@noti0na1 noti0na1

Labels

None yet

Projects

None yet

Milestone

3.8.2

Development

Successfully merging this pull request may close these issues.

Context function parameter name leaks into scope

6 participants

@som-snytt @odersky @noti0na1 @tgodzik @Kordyjan @WojciechMazur