Skip to content

Bringing a symbol to a new run is broken #1895

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

Closed
smarter opened this issue Jan 11, 2017 · 3 comments
Closed

Bringing a symbol to a new run is broken #1895

smarter opened this issue Jan 11, 2017 · 3 comments

Comments

@smarter
Copy link
Member

smarter commented Jan 11, 2017

Summarizing what we discussed last month:
A common pattern in InfoTransformer like ExtensionMethods is to create new symbols like foo$extension in the current phase and to add them to the declarations of a class in the next phase. This causes problem when we try to bring forward the denotation corresponding to such a symbol to a subsequent run, because stillValid considers a denotation to be valid if it's valid in its owner at the first phase it's defined, but at its first phase the symbol has no owner yet!
Here is a testcase:

scala> class Foo(x: Int) extends AnyVal { def foo: Int = 1 }
defined class Foo
scala> new Foo(1).foo
Exception in thread "main" dotty.tools.dotc.core.Denotations$StaleSymbol: stale symbol; method foo$extension#33232 in module class Foo$, defined in Period(12..33, run = 4), is referred to in run Period(13..13, run = 6)
        at dotty.tools.dotc.core.Denotations$SingleDenotation.staleSymbolError(Denotations.scala:932)
...

We can extend stillValid to also check the phase right after the first phase but that's still not enough because InfoTransformers are lazy and the InfoTransformer for the next phase might never have run in the initial run where the symbol was created! I've only been able to reproduce this using the sbt-based bootstrap of dotty by running dotty-compiler-bootstrapped/test-only -- --tests=repl_all. I currently work around this by explicitly forcing the denotation of extension methods at the phase where they're entered in their owner, but this is just a hack.

@DarkDimius suggested that we always recompute denotations from pickling, doing this may require a lot of work.

@smarter smarter changed the title Bringing denotations to new run is broken Bringing a symbol to a new run is broken Jan 11, 2017
smarter added a commit to dotty-staging/dotty that referenced this issue Jan 11, 2017
smarter added a commit to dotty-staging/dotty that referenced this issue Jan 11, 2017
smarter added a commit to dotty-staging/dotty that referenced this issue Jan 11, 2017
smarter added a commit to dotty-staging/dotty that referenced this issue Jan 11, 2017
smarter added a commit to dotty-staging/dotty that referenced this issue Jan 27, 2017
smarter added a commit to dotty-staging/dotty that referenced this issue Jan 27, 2017
smarter added a commit to dotty-staging/dotty that referenced this issue Jan 28, 2017
@odersky
Copy link
Contributor

odersky commented Jan 24, 2018

The test case works now.

@odersky odersky closed this as completed Jan 24, 2018
@smarter
Copy link
Member Author

smarter commented Jan 24, 2018

The test case only works because of the workaround in dotty-staging@51a458e, I believe the underlying problem is still here and could manifest itself in other ways.

@smarter
Copy link
Member Author

smarter commented Jan 24, 2018

Reopening because I think this is important, but feel free to close with stat:revisit

@smarter smarter reopened this Jan 24, 2018
@smarter smarter closed this as completed in 44a305f Jun 4, 2018
smarter added a commit that referenced this issue Jun 4, 2018
Fix #1895: Use proper validity period for extension methods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants