-
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 #13994: initialise inline ctx in lateEnter #14050
Conversation
Before this can be merged we need to add a test case that can reproduce the original issue - sbt scripted tests do not appear to work for this, I have been told that the cmdTests can be useful here |
0f2f837
to
f4d8965
Compare
This is ready to review - the test case reproduces the issue in #13994, requiring the fixes to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need some changes. Detailed review will follow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe evaluate whether some of this can be moved to Namer and be merged with lateEnter
there. It's important to keep driver classes like Run
clean and not burden them with implementation detail.
d2ed41e
to
68f16a0
Compare
@odersky I have moved most of the implementation of |
68f16a0
to
869d359
Compare
869d359
to
39a0f48
Compare
39a0f48
to
573b083
Compare
b3349f0
to
0d7b645
Compare
@smarter I added a comment to the cmd tests, I have also found a way to parse the scala version in bash so I deleted the changes related to loading scala version from java properties. |
@odersky would you like to check this again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that way it is structured better than before.
In
lateEnter
the context is captured in the namer by the inline body annotation incompleteInCreationContext
. So we initialise the context for inlining before callinglateEnter
.I've not figured out a way to replicate this in sbt scripted tests, but you can experiment by copying the sources from #13994 to
library/src-bootstrapped
fixes #13994