Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Skip caret when source is missing in initialization checker #23926

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

Open
liufengyun wants to merge 4 commits into scala:main
base: main
Choose a base branch
Loading
from dotty-staging:fix-init-trace

Conversation

Copy link
Contributor

@liufengyun liufengyun commented Sep 15, 2025

Skip caret when source is missing in initialization checker

@liufengyun liufengyun marked this pull request as ready for review September 15, 2025 12:50
* The method SourceFile#exists always return true thus cannot be used.
*/
def fileExists(source: SourceFile): Boolean =
source.content().nonEmpty
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileContentNonEmpty?

* The method SourceFile#exists always return true thus cannot be used.
*/
def fileExists(source: SourceFile): Boolean =
source.content().nonEmpty
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just headOption,does the nonEmpty read the content?

Copy link
Contributor Author

@liufengyun liufengyun Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean maybe just need to check the first char or line

Copy link
Contributor Author

@liufengyun liufengyun Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

He-Pin reacted with heart emoji
Copy link
Contributor

@olhotak olhotak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 reacted with thumbs up emoji
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olhotak olhotak olhotak approved these changes

+1 more reviewer

@He-Pin He-Pin He-Pin left review comments

Reviewers whose approvals may not affect merge requirements
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /