-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix REPL clashing with CWD artefacts #14021
Conversation
d85f06a
to
a2b6e6a
Compare
This reverts the change that moves all the REPL's artefacts into the "repl$" package, moving everything back to the empty package. Then it fixes the class name and package name shadowing issue, by simulating what the batch compiler does when it compiles a source file without a package statement: it considers the sources to be in the empty package. Then the imports for the previous runs and the user-defined imports will be in sub-contexts of the empty package. The reason the working directory artefacts were shadowing the REPL artefacts was because before the imports were being added to sub-contexts of the root package, afterwhich the REPL tree was being wrapped in the empty package. That meant that the classes found in the empty package (i.e. classfiles in the working directory, or even directories) had precedence over the wildcard imports for the previous runs. The tests now cascade up into contexts that don't have the current compilation unit stored on them, so I had to safeguard against that in Typer. Also, I developed a variant of ShadowingTests, using the batch compiler, to automate the expectations of how the shadowing should and shouldn't work.
a2b6e6a
to
83633a9
Compare
Fixes #12571. Note that having the REPL use a special REPL package isn't necessarily a bad change, but it seemed cleanest to restore the status quo, since there's no longer a specific motivation to keep the change. |
We also have to resolve artefacts clashing with artifacts. This was an interesting discussion which I hope to understand. If artefacts are namespaced by line objects in the empty package, then it's similar to a Scala 2 idea to make the wrapper objects the package objects for the line. I guess the current |
I agree that there's no need to keep the |
@dwijnand Nice detective work btw! |
This reverts the change that moves all the REPL's artefacts into the
"repl$" package (#12607), moving everything back to the empty package.
Then it fixes the class name and package name shadowing issue, by
simulating what the batch compiler does when it compiles a source file
without a package statement: it considers the sources to be in the empty
package. Then the imports for the previous runs and the user-defined
imports will be in sub-contexts of the empty package.
The reason the working directory artefacts were shadowing the REPL
artefacts was because before the imports were being added to
sub-contexts of the root package, afterwhich the REPL tree was being
wrapped in the empty package. That meant that the classes found in the
empty package (i.e. classfiles in the working directory, or even
directories) had precedence over the wildcard imports for the previous
runs.
The tests now cascade up into contexts that don't have the current
compilation unit stored on them, so I had to safeguard against that in
Typer.
Also, I developed a variant of ShadowingTests, using the batch compiler,
to automate the expectations of how the shadowing should and shouldn't
work.