Skip caret when source is missing in initialization checker #23926
Conversation
liufengyun
force-pushed
the
fix-init-trace
branch
from
fb848cd to
2cb6965
Compare
| * The method SourceFile#exists always return true thus cannot be used. | ||
| */ | ||
| def fileExists(source: SourceFile): Boolean = | ||
| source.content().nonEmpty |
| * The method SourceFile#exists always return true thus cannot be used. | ||
| */ | ||
| def fileExists(source: SourceFile): Boolean = | ||
| source.content().nonEmpty |
There was a problem hiding this comment.
Should we just headOption，does the nonEmpty read the content？
There was a problem hiding this comment.
SourceFile#content is a special method with cache. If it is not empty, we will need to read lines from it (thus force loading the content).
There was a problem hiding this comment.
I mean maybe just need to check the first char or line
There was a problem hiding this comment.
I see what you mean. If you check the overall design for code caching and SourceFile#content, you will see there is no need/opportunity for optimization here.
olhotak
left a comment
olhotak
left a comment
There was a problem hiding this comment.
LGTM
In CompilationTests.scala, we are accumulating quite a few ad hoc cases where we compile some files somehow, and later compile some other files together with the output of the first compilation. As a future issue/PR, we should think about ways to factor out any commonality to clean this up.
liufengyun
force-pushed
the
fix-init-trace
branch
from
bac5c81 to
4079628
Compare
We forgot to merge this PR. We can think about the refactoring later: as the code is short & subtle, we need to make sure code reuse/abstraction does not harm maintainability. |
|
@hamzaremmal This PR is ready, please merge it if it does not cause potential problem with release schedule. Thank you. |
3 participants
Skip caret when source is missing in initialization checker