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

Constructor proxy is restricted if class is protected #22563

Merged
merged 2 commits into from
Feb 17, 2025

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Feb 9, 2025

Calling a constructor proxy should be at least as restricted as using new.

Add Protected to a constructor companion if the class is protected.

If the companion exists but is not protected, then make the constructor proxy protected.

Explicit new already errored for access.

Fixes #22560

@som-snytt som-snytt force-pushed the issue/22560-protected-ctor-proxy branch from 2388ac7 to eecbac3 Compare February 10, 2025 01:50
@som-snytt som-snytt marked this pull request as ready for review February 10, 2025 01:59
@Gedochao Gedochao requested a review from odersky February 10, 2025 09:44
@som-snytt som-snytt force-pushed the issue/22560-protected-ctor-proxy branch from eecbac3 to 8c5f1f0 Compare February 13, 2025 02:05
@som-snytt
Copy link
Contributor Author

som-snytt commented Feb 13, 2025

Silly how-I-fixed-it parody:

How I fixed it: I searched github for something similar Prof Odersky had fixed, and I tried to copy that. I saw some bit flags, and since I miss coding in C, I thought I'd try that. It didn't work but threw an exception. It's way past midnight because I had an extra coffee this afternoon. I've already done the word puzzles for tomorrow. I said I better use the class symbol to check if it's protected, so I did that and the test passed, so that's when I stopped. The test is just the OP, plus a line to show something else that doesn't work and something else that does.

Edit: just because it's a parody doesn't mean it's not true.

The follow-up to add a pos test was not a quickie and took some time, per the next comment. I would have liked visibility ("instrumentation") into operations on symbols and trees, but lacking that, excellent naming in the code base was very helpful.

@som-snytt som-snytt marked this pull request as draft February 13, 2025 02:30
@som-snytt
Copy link
Contributor Author

Refresher that qualified protected does not mean access is granted to subclasses of the privateWIthin boundary symbol.

The expectation is that this works:

class Enumeration:
  protected class Val(i: Int):
    def this() = this(42)
  object Val:
    protected[Enumeration] def apply(i: Int) = new Val(i)

class Test extends Enumeration:
  val Hearts = Val(27)
  val Diamonds = Val()

Scala 2 has the same behavior.

An alternative is to require the companion to have the same access modifiers (that is, also be protected for this use case).

@som-snytt som-snytt changed the title Constructor proxy is protected if class is Constructor proxy is restricted if class is protected Feb 13, 2025
Require the constructor companion to be protected if the class is;
or create it protected. Otherwise, create protected proxies.
@som-snytt som-snytt force-pushed the issue/22560-protected-ctor-proxy branch from 8c5f1f0 to fddab10 Compare February 13, 2025 08:24
@som-snytt
Copy link
Contributor Author

The ❌ is an unrelated server failure.

}
case _ =>
tryInsertApplyOrImplicit(tree, pt, locked):
errorTree(tree, MethodDoesNotTakeParameters(tree))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This delta was due to a println. Philosophically, tryInsert is the important part, and putting it right of the arrow makes it less visible, and unduly highlights errorTree. The edit with new syntax is clearer.

Usual idiom where "thing to the right" is less important for understanding because it doesn't carry much information:

def f(x: X) = x match
  case _: Y =>

@som-snytt som-snytt marked this pull request as ready for review February 14, 2025 17:36
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

LGTM

@odersky odersky merged commit 705e10e into scala:main Feb 17, 2025
29 checks passed
@som-snytt som-snytt deleted the issue/22560-protected-ctor-proxy branch February 17, 2025 17:22
@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.

Objects leak protected inner classes
3 participants