Skip to content

Commit

Permalink
Add clause for protected visibility from package objects
Browse files Browse the repository at this point in the history
We usually have an access rule that the access to a protected member `foo` in class `C`
must be from somewhere nested in a subclass of `C`. But that fails if the member is
accessed from a package object `p.package`. In that case, the access does not need to be in
the same object, it just has to be in package `p`. This clause was previously missing
and is now added.

Why was this only recently discovered?

#18057 fixed an issue where toplevel protected members were always accessible
because explicit package object prefixes were added after the accessibility check was done,
and would re-establish the previous members without doing an accessibility check. The fix was
done by adding package objects first, then doing he rest of the checks. But that also means
that protected toplevel objects now get checked as members of their synthetic package object
instead of as members of their package.

The change here also makes specs2 compile again.
  • Loading branch information
odersky committed Jul 4, 2023
1 parent 0a21ecf commit 6793945
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,7 @@ class CommunityBuildTestC:
@Test def sconfig = projects.sconfig.run()
@Test def shapeless = projects.shapeless.run()
@Test def sourcecode = projects.sourcecode.run()

// Disabled. Currently fails in FutureMatchers.scala. The call to
// `checkResultFailure` goes to a protected method which is not accessible.
// I tried to fix it, but get test failures.
// @Test def specs2 = projects.specs2.run()
@Test def specs2 = projects.specs2.run()

@Test def stdLib213 = projects.stdLib213.run()
@Test def ujson = projects.ujson.run()
Expand Down
11 changes: 7 additions & 4 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -907,10 +907,13 @@ object SymDenotations {
false
val cls = owner.enclosingSubClass
if !cls.exists then
val encl = if ctx.owner.isConstructor then ctx.owner.enclosingClass.owner.enclosingClass else ctx.owner.enclosingClass
fail(i"""
| Access to protected $this not permitted because enclosing ${encl.showLocated}
| is not a subclass of ${owner.showLocated} where target is defined""")
if pre.termSymbol.isPackageObject && accessWithin(pre.termSymbol.owner) then
true
else
val encl = if ctx.owner.isConstructor then ctx.owner.enclosingClass.owner.enclosingClass else ctx.owner.enclosingClass
fail(i"""
| Access to protected $this not permitted because enclosing ${encl.showLocated}
| is not a subclass of ${owner.showLocated} where target is defined""")
else if isType || pre.derivesFrom(cls) || isConstructor || owner.is(ModuleClass) then
// allow accesses to types from arbitrary subclasses fixes #4737
// don't perform this check for static members
Expand Down
15 changes: 15 additions & 0 deletions tests/pos/i18124/definition.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// definition.scala
package oolong.bson:

trait BsonValue
protected def merge(
base: BsonValue,
patch: BsonValue,
arraySubvalues: Boolean = false
): BsonValue = ???

private def foo: Int = 1

package inner:
protected[bson] def bar = 2

8 changes: 8 additions & 0 deletions tests/pos/i18124/usage.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// usage.scala
package oolong.bson

extension (bv: BsonValue)
def :+(other: BsonValue): BsonValue = merge(other, bv, false)

val x = foo
val y = inner.bar

0 comments on commit 6793945

Please sign in to comment.