Skip to content

Make sure lazy accessors in traits are not private. #1164

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

Jump to bottom
Merged
merged 6 commits into from
Mar 13, 2016
Merged

Make sure lazy accessors in traits are not private. #1164

merged 6 commits into from
Mar 13, 2016

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 10, 2016

Fixes #1140. Review by @DarkDimius or @smarter.

odersky added 3 commits March 10, 2016 11:30
@odersky
Make sure lazy accessors in traits are not private.
b6daa2a 
Fixes scala#1140. Review by @DarkDimius or @smarter.
@odersky
More detailed reporting in TreeChecker
9ae558a 
... when definitions are missing.
@odersky
Fix and simplify initializer
df5fcef 
Initializer was needlessly complex and did not work anymore
for lazy vals (for them, we implicitly made use of the fact that
the initializer would find the symbol itself. But after name
mangling that logic would break down.
@smarter
Copy link
Member

smarter commented Mar 12, 2016

So, why was separate compilation needed to reproduce the bug? Could we avoid special-casing non-separate compilation to make it easier to find bugs like this?

@odersky
Copy link
Contributor Author

odersky commented Mar 12, 2016

There are two kinds of tests for making things non-private. One kind makes things not private if they are referenced from some context that demands it. The other makes them not private unconditionally. The failing case was overlooked in the second kind of tests, but still showed up in the first kind. That's why
the error only showed up under separate compilation.

@smarter
Copy link
Member

smarter commented Mar 12, 2016

Could we make ExpandPrivate#ensurePrivateAccessible assert that the denotation we're making public comes from the current compilation unit? If it comes from a different compilation unit and ensurePrivateAccessible is needed, then separate compilation will always fail, won't it?

odersky added 3 commits March 13, 2016 15:06
@odersky
Propagate source file to synthetic companions
d500b1d
@odersky
Assert that ensureNotPrivate does not break on separate compilation
9daf976 
When ensureNotPrivate changes the status of a formerly private declaration,
assert that the reference to the declaration is in the same compilation unit,
as otherwise the nehavior would be different under separate compilation.
@odersky
Make Synthetic a FormStart flag.
6b983e3 
Not needed in the end for this patch, but anyway a good idea.
@odersky
Copy link
Contributor Author

odersky commented Mar 13, 2016

Could we make expandPrivate#ensurePrivateAccessible assert that the denotation we're making public comes from the current compilation unit?

Good idea. I added commits to do this.

@smarter
Copy link
Member

smarter commented Mar 13, 2016

Nice! LGTM.

smarter added a commit that referenced this pull request Mar 13, 2016
@smarter
Merge pull request #1164 from dotty-staging/fix-#1140
daffba9 
Make sure lazy accessors in traits are not private.
@smarter smarter merged commit daffba9 into scala:master Mar 13, 2016
@allanrenucci allanrenucci deleted the fix-#1140 branch December 14, 2017 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@odersky @smarter