Skip to content

Fix-#1723: Avoid private leaks on completion #1922

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.

Sign up for GitHub

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 3 commits into from
Feb 2, 2017
Merged

Fix-#1723: Avoid private leaks on completion #1922

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1a37e1
Fix-#1723: Avoid private leaks on completion
odersky Feb 1, 2017
157c8dc
Refactoring to make Checking not mutate denotations
odersky Feb 1, 2017
d64d0a0
Fix typo
odersky Feb 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,11 +733,7 @@ class TreeUnpickler(reader: TastyReader, tastyName: TastyName.Table, posUnpickle
// no longer necessary.
goto(end)
setPos(start, tree)

// This is also done in PostTyper but needs to be redone here
if (!sym.is(SyntheticOrPrivate) && sym.owner.isClass) {
sym.info = Checking.checkNoPrivateLeaks(sym, tree.pos)
}
Checking.avoidPrivateLeaks(sym, tree.pos)
tree
}

Expand Down
6 changes: 0 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/PostTyper.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,6 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
private def transformMemberDef(tree: MemberDef)(implicit ctx: Context): Unit = {
val sym = tree.symbol
sym.transformAnnotations(transformAnnot)
// Has to be redone in TreeUnpickler
if (!sym.is(SyntheticOrPrivate) && sym.owner.isClass) {
val info1 = Checking.checkNoPrivateLeaks(sym, tree.pos)
if (info1 ne sym.info)
sym.copySymDenotation(info = info1).installAfter(thisTransformer)
}
}

private def transformSelect(tree: Select, targs: List[Tree])(implicit ctx: Context): Tree = {
Expand Down
5 changes: 5 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ object Checking {
fail(i"$sym cannot have the same name as ${cls.showLocated} -- class definitions cannot be overridden")
sym.setFlag(Private) // break the overriding relationship by making sym Private
}
avoidPrivateLeaks(sym, sym.pos)
Copy link
Member

Choose a reason for hiding this comment

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

So far, no method in Checking mutates anything, it would be nice to keep this pattern. I suggest doing what checkNonCyclic does and returning the potentially updated sym.info, i.e.:

if (!sym.is(SyntheticOrPrivate) && sym.owner.isClass)
  checkNoPrivateLeaks(sym, sym.pos)
else
  sym.info

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So far, no method in Checking mutates anything

The line just above the avoidPrivateLeaks call is also a mutation (admittedly, only in the error case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have now refactored. You were right, the new organization is more coherent.

}

/** Check the type signature of the symbol `M` defined by `tree` does not refer
Expand Down Expand Up @@ -412,6 +413,10 @@ object Checking {
info
}

def avoidPrivateLeaks(sym: Symbol, pos: Position)(implicit ctx: Context) =
if (!sym.is(SyntheticOrPrivate) && sym.owner.isClass)
sym.info = checkNoPrivateLeaks(sym, pos)

/** Verify classes extending AnyVal meet the requirements */
def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context) = {
def checkValueClassMember(stat: Tree) = stat match {
Expand Down
8 changes: 8 additions & 0 deletions tests/pos/i1723.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class A {
private val x: List[Int] = List(1)
def foo = x.head // foo inferred type is this.x.scala$collection$immutable$List$$A
}

class B extends A {
foo
}