8000 Allow companion access boundary · scala/scala@3fedd33 · GitHub
[go: up one dir, main page]
Skip to content

Commit 3fedd33

Browse files
som-snyttsom-snytt
committed
Allow companion access boundary
When overriding `protected[C]`, `C` may refer to the enclosing companion of `C`, which provides the same access guarantees. Update spec to allow override protected[C] companion.
1 parent 2e24de0 commit 3fedd33

File tree

15 files changed

+470
-150
lines changed

15 files changed

+470
-150
lines changed

‎spec/05-classes-and-objects.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,13 +332,13 @@ Furthermore, the following restrictions on modifiers apply to ´M´ and
332332

333333
- ´M'´ must not be labeled `final`.
334334
- ´M´ must not be [`private`](#modifiers).
335-
- If ´M´ is labeled `private[´C´]` for some enclosing class or package ´C´,
336-
then ´M'´ must be labeled `private[´C'´]` for some class or package ´C'´ where
337-
´C'´ equals ´C´ or ´C'´ is contained in ´C´.
338-
339-
<!-- TODO: check whether this is accurate -->
340335
- If ´M´ is labeled `protected`, then ´M'´ must also be
341336
labeled `protected`.
337+
- If ´M´ is labeled `private[´C´]` (respectively `protected[´C´]`)
338+
for some enclosing class or package ´C´,
339+
then ´M'´ must be labeled `private[´C'´]` (or, respectively, `protected[´C'´]`)
340+
for some class or package ´C'´ where
341+
´C'´ equals ´C´ or the companion of ´C´, or ´C'´ is contained in ´C´.
342342
- If ´M'´ is not an abstract member, then ´M´ must be labeled `override`.
343343
Furthermore, one of two possibilities must hold:
344344
- either ´M´ is defined in a subclass of the class where is ´M'´ is defined,

‎src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -354,37 +354,44 @@ abstract class RefChecks extends Transform {
354354
overrideErrorWithMemberInfo("`override` modifier required to override concrete member:")
355355
}
356356

357-
def overrideAccessError(): Unit = {
358-
val otherAccess = accessFlagsToString(other)
359-
overrideError("weaker access privileges in overriding\n"+
360-
overriddenWithAddendum(s"override should ${(if (otherAccess == "") "be public" else "at least be " + otherAccess)}"))
361-
}
357+
def weakerAccessError(advice: String): Unit = overrideError(s"weaker access privileges in overriding\n${overriddenWithAddendum(advice)}")
358+
def overrideAccessError(): Unit =
359+
accessFlagsToString(other) match {
360+
case "" => weakerAccessError("override should be public")
361+
case otherAccess => weakerAccessError(s"override should at least be $otherAccess")
362+
}
362363

363364
//Console.println(infoString(member) + " overrides " + infoString(other) + " in " + clazz);//DEBUG
364365

365366
/* Is the intersection between given two lists of overridden symbols empty? */
366-
def intersectionIsEmpty(syms1: List[Symbol], syms2: List[Symbol]) =
367-
!(syms1 exists (syms2 contains _))
367+
def intersectionIsEmpty(syms1: List[Symbol], syms2: List[Symbol]) = !syms1.exists(syms2 contains _)
368368

369-
if (memberClass == ObjectClass && otherClass == AnyClass) {} // skip -- can we have a mode of symbolpairs where this pair doesn't even appear?
369+
if (memberClass == ObjectClass && otherClass == AnyClass) () // skip -- can we have a mode of symbolpairs where this pair doesn't even appear?
370370
else if (typesOnly) checkOverrideTypes()
371371
else {
372372
// o: public | protected | package-protected (aka java's default access)
373373
// ^-may be overridden by member with access privileges-v
374374
// m: public | public/protected | public/protected/package-protected-in-same-package-as-o
375375

376376
if (member.isPrivate) // (1.1)
377-
overrideError("weaker access privileges in overriding\n"+
378-
overriddenWithAddendum(s"override should not be private"))
377+
overrideError(sm"""weaker access privileges in overriding
378+
|${overriddenWithAddendum("override should not be private")}""")
379379

380380
// todo: align accessibility implication checking with isAccessible in Contexts
381-
val ob = other.accessBoundary(memberClass)
382-
val mb = member.accessBoundary(memberClass)
383-
def isOverrideAccessOK = member.isPublic || { // member is public, definitely same or relaxed access
384-
(!other.isProtected || member.isProtected) && // if o is protected, so is m
385-
((!isRootOrNone(ob) && ob.hasTransOwner(mb)) || // m relaxes o's access boundary
386-
(other.isJavaDefined && other.isProtected)) // overriding a protected java member, see #3946 #12349
381+
@inline def protectedOK = !other.isProtected || member.isProtected
382+
@inline def accessBoundaryOK = {
383+
val ob = other.accessBoundary(memberClass)
384+
val mb = member.accessBoundary(memberClass)
385+
@inline def companionBoundaryOK = ob.isClass && mb.isModuleClass && mb.module == ob.companionSymbol
386+
!isRootOrNone(ob) && (ob.hasTransOwner(mb) || companionBoundaryOK)
387387
}
388+
@inline def otherIsJavaProtected = other.isJavaDefined && other.isProtected
389+
def isOverrideAccessOK =
390+
member.isPublic || // member is public, definitely same or relaxed access
391+
protectedOK && // if o is protected, so is m
392+
(accessBoundaryOK || // m relaxes o's access boundary
393+
otherIsJavaProtected // overriding a protected java member, see #3946 #12349
394+
)
388395
if (!isOverrideAccessOK) {
389396
overrideAccessError()
390397
} else if (other.isClass) {
@@ -753,10 +760,8 @@ abstract class RefChecks extends Transform {
753760
lazy val varargsType = toJavaRepeatedParam(member.tpe)
754761

755762
def isSignatureMatch(sym: Symbol) = !sym.isTerm || {
756-
val symtpe = clazz.thisType memberType sym
757-
def matches(tp: Type) = tp matches symtpe
758-
759-
matches(member.tpe) || (isVarargs && matches(varargsType))
763+
val symtpe = clazz.thisType.memberType(sym)
764+
member.tpe.matches(symtpe) || (isVarargs && varargsType.matches(symtpe))
760765
}
761766
/* The rules for accessing members which have an access boundary are more
762767
* restrictive in java than scala. Since java has no concept of package nesting,
@@ -781,15 +786,19 @@ abstract class RefChecks extends Transform {
781786
|| sym.isProtected // marked protected in java, thus accessible to subclasses
782787
|| sym.privateWithin == member.enclosingPackageClass // exact package match
783788
)
784-
def classDecls = inclazz.info.nonPrivateDecl(member.name)
785-
def matchingSyms = classDecls filter (sym => isSignatureMatch(sym) && javaAccessCheck(sym))
789+
def classDecl =
790+
inclazz.info.nonPrivateDecl(member.name) match {
791+
case NoSymbol => inclazz.info.nonPrivateDecl(member.unexpandedName)
792+
case exact => exact
793+
}
794+
def matchingSyms = classDecl.filter(sym => isSignatureMatch(sym) && javaAccessCheck(sym))
786795

787796
(inclazz != clazz) && (matchingSyms != NoSymbol)
788797
}
789798

790799
// 4. Check that every defined member with an `override` modifier overrides some other member.
791800
for (member <- clazz.info.decls)
792-
if (member.isAnyOverride && !(clazz.thisType.baseClasses exists (hasMatchingSym(_, member)))) {
801+
if (member.isAnyOverride && !(clazz.thisType.baseClasses.exists(hasMatchingSym(_, member)))) {
793802
// for (bc <- clazz.info.baseClasses.tail) Console.println("" + bc + " has " + bc.info.decl(member.name) + ":" + bc.info.decl(member.name).tpe);//DEBUG
794803

795804
val nonMatching: List[Symbol] = clazz.info.member(member.name).alternatives.filterNot(_.owner == clazz).filterNot(_.isFinal)

‎src/compiler/scala/tools/nsc/typechecker/SuperAccessors.scala

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,14 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT
117117
atPos(sel.pos)(Select(gen.mkAttributedThis(clazz), superAcc) setType sel.tpe)
118118
}
119119

120-
private def transformArgs(params: List[Symbol], args: List[Tree]) = {
120+
private def transformArgs(params: List[Symbol], args: List[Tree]) =
121121
treeInfo.mapMethodParamsAndArgs(params, args) { (param, arg) =>
122122
if (isByNameParamType(param.tpe))
123123
withInvalidOwner(transform(arg))
124124
else transform(arg)
125125
}
126-
}
127126

128-
/** Check that a class and its companion object to not both define
127+
/** Check that a class and its companion object do not both define
129128
* a class or module with same name
130129
*/
131130
private def checkCompanionNameClashes(sym: Symbol) =
@@ -136,9 +135,8 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT
136135
if (other == NoSymbol)
137136
other = linked.info.decl(sym.name.toTermName).filter(_.isModule)
138137
if (other != NoSymbol)
139-
reporter.error(sym.pos, "name clash: "+sym.owner+" defines "+sym+
140-
"\nand its companion "+sym.owner.companionModule+" also defines "+
141-
other)
138+
reporter.error(sym.pos, sm"""name clash: ${sym.owner} defines $sym
139+
|and its companion ${sym.owner.companionModule} also defines $other""")
142140
}
143141
}
144142

@@ -240,13 +238,13 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT
240238
val decls = sym.info.decls
241239
for (s <- decls) {
242240
val privateWithin = s.privateWithin
243-
if (privateWithin.isClass && !s.hasFlag(EXPANDEDNAME | PROTECTED) && !privateWithin.isModuleClass &&
244-
!s.isConstructor) {
241+
def isPrivateWithinNonCompanionModule = privateWithin.isModuleClass
242+
if (privateWithin.isClass && !s.hasFlag(EXPANDEDNAME | PROTECTED) && !isPrivateWithinNonCompanionModule && !s.isConstructor) {
245243
val savedName = s.name
246244
decls.unlink(s)
247245
s.expandName(privateWithin)
248246
decls.enter(s)
249-
log("Expanded '%s' to '%s' in %s".format(savedName, s.name, sym))
247+
log(f"Expanded '$savedName' to '${s.name}' in $sym")
250248
}
251249
}
252250
super.transform(tree)
@@ -303,23 +301,15 @@ abstract class SuperAccessors extends transform.Transform with transform.TypingT
303301
// also exists in a superclass, because they may be surprised
304302
// to find out that a constructor parameter will shadow a
305303
// field. See scala/bug#4762.
306-
if (settings.warnPrivateShadow) {
307-
if (sym.isPrivateLocal && sym.paramss.isEmpty) {
308-
qual.symbol.ancestors foreach { parent =>
309-
parent.info.decls filterNot (x => x.isPrivate || x.isLocalToThis) foreach { m2 =>
310-
if (sym.name == m2.name && m2.isGetter && m2.accessed.isMutable) {
311-
runReporting.warning(sel.pos,
312-
sym.accessString + " " + sym.fullLocationString + " shadows mutable " + m2.name
313-
+ " inherited from " + m2.owner + ". Changes to " + m2.name + " will not be visible within "
314-
+ sym.owner + " - you may want to give them distinct names.",
315-
WarningCategory.LintPrivateShadow,
316-
currentOwner)
317-
}
318-
}
319-
}
320-
}
321-
}
322-
304+
if (settings.warnPrivateShadow && sym.isPrivateLocal && sym.paramss.isEmpty)
305+
for (parent <- qual.symbol.ancestors)
306+
for (m2 <- parent.info.decls)
307+
if (!m2.isPrivate && !m2.isLocalToThis && sym.name == m2.name && m2.isGetter && m2.accessed.isMutable)
308+
runReporting.warning(sel.pos,
309+
sq"""${sym.accessString} ${sym.fullLocationString} shadows mutable ${m2.name} inherited from ${m2.owner}.
310+
> Changes to ${m2.name} will not be visible within ${sym.owner}; you may want to give them distinct names.""",
311+
WarningCategory.LintPrivateShadow,
312+
currentOwner)
323313

324314
def isAccessibleFromSuper(sym: Symbol) = {
325315
val pre = SuperType(sym.owner.tpe, qual.tpe)

‎src/reflect/scala/reflect/internal/util/StripMarginInterpolator.scala

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,31 +23,38 @@ trait StripMarginInterpolator {
2323
* and [[scala.StringContext#raw]].
2424
*
2525
* The margin of each line is defined by whitespace leading up to a '|' character.
26-
* This margin is stripped '''before''' the arguments are interpolated into to string.
26+
* This margin is stripped '''before''' the arguments are interpolated into the string.
2727
*
2828
* String escape sequences are '''not''' processed; this interpolator is designed to
2929
* be used with triple quoted Strings.
3030
*
3131
* {{{
3232
* scala> val foo = "f|o|o"
3333
* foo: String = f|o|o
34-
* scala> sm"""|${foo}
34+
* scala> sm"""|${foo}|
3535
* |"""
3636
* res0: String =
37-
* "f|o|o
37+
* "f|o|o|
3838
* "
3939
* }}}
4040
*/
41-
final def sm(args: Any*): String = {
41+
final def sm(args: Any*): String = impl('|', args: _*)
42+
43+
private final def impl(sep: Char, args: Any*): String = {
4244
def isLineBreak(c: Char) = c == '\n' || c == '\f' // compatible with StringOps#isLineBreak
4345
def stripTrailingPart(s: String) = {
4446
val (pre, post) = s.span(c => !isLineBreak(c))
45-
pre + post.stripMargin
47+
pre + post.stripMargin(sep)
4648
}
4749
val stripped: List[String] = stringContext.parts.toList match {
48-
case head :: tail => head.stripMargin :: (tail map stripTrailingPart)
50+
case head :: tail => head.stripMargin(sep) :: tail.map(stripTrailingPart)
4951
case Nil => Nil
5052
}
5153
new StringContext(stripped: _*).raw(args: _*)
5254
}
55+
56+
/** Like the `sm` interpolator, but strips quotation-style delimiter `>`
57+
* and merges the resulting lines into a single line string.
58+
*/
59+
final def sq(args: Any*): String = impl('>', args: _*).linesIterator.mkString
5360
}

0 commit comments

Comments
 (0)
0