-
Notifications
You must be signed in to change notification settings - Fork 272
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
multiple-round: implement incremental providers #1494
Conversation
and replace StandaloneAnalysisAPISessionBuilder, which hides KotlinCoreProjectEnvironment, which is needed to update KotlinCliJavaFileManagerImpl for new files.
a0ec509
to
ddc179b
Compare
Also, do not create processors repeatedly in each round.
|
||
override fun toResult(): List<String> { | ||
return visitor.toResult() | ||
return result |
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.
what's this change for?
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.
Some KtSymbols are not available after cache drop
import org.jetbrains.kotlin.psi.KtScript | ||
import org.jetbrains.kotlin.psi.KtTypeAlias | ||
|
||
class IncrementalKotlinDeclarationProvider(var del: KotlinDeclarationProvider) : KotlinDeclarationProvider() { |
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.
if we are purely delegating here, it might make more sense to simply use delegation without need to write down all the redundant overrides here.
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.
Unfortunately, by
delegation doesn't allow swapping the delegate.
lateinit var processors: List<SymbolProcessor> | ||
lateinit var newKSFiles: List<KSFile> | ||
lateinit var newFileNames: Set<String> | ||
while (!finished) { |
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.
are we falling back to the previous design for multiple round processing? Just want to know if this conflicts with the plan to do multiple round within Gradle runner.
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.
IIRC the plan is to do it here, in the unified entry point of production and test. There will be multilple callers in addition to gradle and we want the setup to be as easy as possible.
and replace StandaloneAnalysisAPISessionBuilder, which hides KotlinCoreProjectEnvironment, which is needed to update KotlinCliJavaFileManagerImpl for new files.