Skip to content
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

SI-6623 -Yrepl-use-magic-imports avoids nesting $iw wrappers #79

Closed
wants to merge 24 commits into from

Conversation

retronym
Copy link
Owner

@retronym retronym commented Dec 3, 2019

Rather than adding a wrapper object for each import in the session history,
just use a single wrapper preceded by the imports which have been
interspersed with a magic import to bump context depth.

Code is still ordinarily wrapped in a $read object.

This is a step toward 6623-like transparency.

retronym takes the blame for this innovation.
adriaanm collaborated in its commission.
somsnytt batted clean-up.

retronym and others added 12 commits October 31, 2019 10:53
(For 2.12.x's eyes only.)

Use vals to cache a single instance of stateless CanBuildFrom instances.
These are cast by the existing `implicit def` to the suitable generic
type. This pattern was already used in some places -- this PR applies
it systematically across `collection.{mutable,immutable}`.

The `CanBuildFrom` instances for arrays and wrapped arrays are
cached for each primitive type, Unit, and Object. Each of these
instances is backed by a dedicated subclass of CanBuildFrom that
avoids subsequent dispatch on the `ClassTag[T]`.
[nomerge] Backport windows test fix
order matches in expected frequency order (Array, WrappedArray + associated builders, and ClassTag.newArray)
avoid extra def in BitSets
don't optimise for NoBuilder cases
two more tests for apparently-fixed issues
[nomerge] ConcatIterator.last advances
[backport] Import travis caching config and JDK install from scala-dev
use direct vals where no casting is required
@retronym retronym force-pushed the review/5220 branch 3 times, most recently from 6f5ae9e to 16778e7 Compare December 5, 2019 07:29
Avoid allocations of reusable CanBuildFroms
The test allocates 16*16 MB, so testing with a 192 MB heap should
be safe to catch the leak.

Tested with running the test in a `while true` loop locally
  - constanly fails with 192 MB on 2.13.1
  - constantly succeeds with 192 MB on 2.13.2-bin-9ef8fc3
  - flaky with 128 MB on 2.13.2-bin-9ef8fc3

(cherry picked from commit ffcffc6)
  - Make `ImportContext` a nested class
  - Avoid the constructor of the superclass `Context` needing to
    call methods on the unitialized subclass ImportContext by
    computing `isRootContext` / `depth` externally in the
    factory method.
Rather than adding a wrapper object for each import in the session
history, just use a single wrapper preceded by the imports which
have been interspersed with a magic import to bump context depth.

Code is still ordinarily wrapped in a `$read` object.

This is a step toward 6623-like transparency.

`retronym` takes the blame for this innovation.
`adriaanm` collaborated in its commission.
`somsnytt` batted clean-up.
… bug

By default, the parser tries to heal incomplete source files by inserting
missing braces. Compilation will still error out, but any subsequent
parser/type errors make more sense to the user when this healing works.

The healing uses indentation to figure out the intent of the code. Wrapped
REPL snippets aren't properly indented, and in the test case I added the
healing seems counterproductive.

This commit disables it in REPL tab completion, in line with the way that
IMain parses the wrapped code for real compilation.
  - add comments describing a problem I discovered with imports
    and a possible solution
  - Make strip wrappers from REPL output for the wrappers generated in this mode
This detail changes under -Yrepl-class-based. We prefer tests that operate
under either mode.
Revert "Enable magic imports unconditionally to survey test results"

This reverts commit b02f890.

Revert "Enable -Yrepl-class-based to survey test failures"

This reverts commit d64af1a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants