Skip to content

Nowarn public implicit val class params #22664

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

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

som-snytt
Copy link
Contributor

Similar to explicit param.

Fixes #22662

@som-snytt som-snytt force-pushed the issue/22662-implicit-val branch from 02cd651 to 0a80777 Compare February 26, 2025 18:07
@som-snytt
Copy link
Contributor Author

som-snytt commented Feb 26, 2025

Explanation of protected member of given class is #8397

The warning considers any such member. But adding Protected is restricted to members the user might name. That may be wrong if a macro def f = summonInline[G].T constructs a usage of the derived name in a leaky position.

@som-snytt som-snytt marked this pull request as ready for review February 26, 2025 21:40
@Gedochao Gedochao requested a review from sjrd February 27, 2025 09:38
@som-snytt
Copy link
Contributor Author

So you're saying that you should only warn on private accessor, except that givens might be widened to protected so also warn about those.

The other tweak, to only widen givens that have a human name, is therefore not strictly needed for warning purposes? if you're willing to warn about protected.

@sjrd
Copy link
Member

sjrd commented Mar 6, 2025

@Gedochao I don't fell qualified to review this. I don't understand how/why there suddenly are protected things in givens, or even how that is supposed to make them "not leak".

@Gedochao Gedochao requested a review from odersky March 6, 2025 14:09
@Gedochao Gedochao assigned odersky and unassigned sjrd Mar 6, 2025
Dowarn protected param accessor of given, which was probably widened.
@som-snytt som-snytt force-pushed the issue/22662-implicit-val branch from 0a80777 to 2cb2c92 Compare March 6, 2025 16:34
@som-snytt
Copy link
Contributor Author

som-snytt commented Mar 6, 2025

@sjrd me neither. I removed that bit, which also didn't have a (new) test. This is just for the hapless warning. May I beg a review?

github doesn't offer the poke widget yet. (I just read a comment from a few years ago along the lines of, if we're swamped with trivial tickets, we can't get real work done.)

@som-snytt som-snytt assigned sjrd and unassigned odersky Mar 6, 2025
@sjrd sjrd enabled auto-merge March 6, 2025 16:42
@sjrd sjrd merged commit 8b04ba8 into scala:main Mar 6, 2025
27 checks passed
@som-snytt som-snytt deleted the issue/22662-implicit-val branch March 6, 2025 18:18
@som-snytt
Copy link
Contributor Author

@sjrd merci beaucoup.

@WojciechMazur WojciechMazur added this to the 3.7.0 milestone Mar 11, 2025
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.

Don't warn using val
4 participants