Skip to content

Infer private[this] #7411

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 10 commits into from
Oct 21, 2019
Merged

Infer private[this] #7411

merged 10 commits into from
Oct 21, 2019

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Oct 12, 2019

Infer private[this] if all accesses are via this, even if it is not explicitly specified.

Deprecate usage of private[this] and protected[this] under -strict (i.e. for 3.1.).

The main reasons for dropping private[this] are:

  • it is syntactically an irregular case. A pair of brackets usually encloses a type, but this is a value.
  • its effect over private is purely local and can be easily inferred
  • it leads to bike shedding: should I use private or private[this]? One is shorter but the other might be more efficient.

protected[this] by now influences compiler decisions in no way at all. Its delta over protected is small. protected by itself is niche (e.g. Don Syme has argued eloquently why it is too niche for F#), so having two variants of it looks like overkill.

The reason is that some of these fields might be converted later to private[this]
fields which do not get  setter. Instead, generate setters for private vars
that are not private[this] in phase Getters.
The plan is that by then we know when a members is private[this].
@odersky odersky marked this pull request as ready for review October 13, 2019 14:45
@odersky
Copy link
Contributor Author

odersky commented Oct 13, 2019

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/7411/ to see the changes.

Benchmarks is based on merging with master (0f7a85b)

@pshirshov
Copy link
Contributor

pshirshov commented Oct 13, 2019

While I welcome removal of this syntax, I don't thin that the semantic is unneccessary. It's very convenient to be able to enforce stricter rules. Also I wouldn't call protected modifier a niche thing - more access control is always better.

To be quiet honest I think we need lot more flexible access restriction system, so we may specifiy complex predicates on who may and may not access an entity.

Could you consider keeping the functionality under different syntax please?.. Maybe not a modifer but an annotation?..

@odersky
Copy link
Contributor Author

odersky commented Oct 14, 2019

Maybe not a modifer but an annotation?..

That's just a cop out. Seriously, I have not seen a convincing argument why distinctions between private/protected and private/protected[this] are necessary. And unnecessary distinctions just lead to bikeshedding and complexity.

One could argue that the default is wrong and it should always be [this]. We already had this discussion, and I believe that the reasons for keeping plain private and protected were convincing, for algorithmic reasons as well as backwards compatibility.

@anatoliykmetyuk anatoliykmetyuk self-requested a review October 15, 2019 11:38
@anatoliykmetyuk anatoliykmetyuk self-assigned this Oct 15, 2019
@anatoliykmetyuk anatoliykmetyuk merged commit 0bf296e into scala:master Oct 21, 2019
@anatoliykmetyuk anatoliykmetyuk deleted the drop-local branch October 21, 2019 13:08
@anatoliykmetyuk anatoliykmetyuk added this to the 0.20 Tech Preview milestone Oct 31, 2019
@neko-kai
Copy link
Contributor

neko-kai commented Oct 31, 2019

@odersky
protected[this] and [this] access modifiers in general are required for variance in path-dependent types, e.g.

trait ProtectedThisVarianceOk[-A, +B] {
  protected[this] type AliasA = A
  protected[this] type AliasB = B
}

trait ProtectedVarianceIsNotOk[-A, +B] {
  // Error:(16, 18) contravariant type A occurs in invariant position in type A of type AliasA
  //  protected type AliasA = A
  protected type AliasA = A
  protected type AliasB = B
}

Scala 2 allows declaration of AliasA because it's not a visible type member of ProtectedThisVarianceOk – as an internal-only alias its aliasing of contravariant A cannot break the type's outer visible variance.

IMHO removing protected[this] means fundamentally gimping path-dependency.

@odersky
Copy link
Contributor Author

odersky commented Nov 2, 2019

Treating protected[this] specially for variance is unfortunately unsound (scala/bug#7093). Nothing changes for private[this]; the [this] is simply inferred.

@neko-kai
Copy link
Contributor

neko-kai commented Nov 3, 2019

Sorry, I was not aware. It still feels like a bad blow for cake & PDT; possibly less dramatic if variant aliases are out of the picture anyway... Removal of private[this] is of course strictly an improvement (if someone needs a not-this private for reflective field access, they can use private[MyClass] for the same effect)

@ryanberckmans
Copy link

Here is a proposal to retain private[this] in Scala 3.

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.

6 participants