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

Filter index entries for deleted source files #1194

Merged
merged 1 commit into from May 2, 2024

Conversation

ahoppen
Copy link
Collaborator

@ahoppen ahoppen commented Apr 20, 2024

This introduces an abstraction layer around IndexStoreDB (yes, I known another one …) that consults the underlying IndexStoreDB and then checks whether the entries are up-to-date with respect to some IndexCheckLevel. Requests can specify how picky they want to be about declaring results from the index out-of-date.

The default choice right now is to not include index entries from source files that have been deleted (because those are definitely from lingering unit files) but include results even if the source file has been modified after it was last indexed. We do include results from files that have been modified since they were last indexed because: When a file gets modified, it's likely that some of the line:column locations in it are still correct – eg. if only one line is modified and if lines are inserted/deleted all locations above are still correct. For locations that are out of date, showing stale results is one of the best ways of communicating to the user that the index is out-of-date and that they need to rebuild. We might want to reconsider this default when we have background indexing.

rdar://125230833
rdar://126622963

@ahoppen ahoppen force-pushed the check-index-up-to-date branch 2 times, most recently from 4aade6c to de48289 Compare April 24, 2024 15:33
@ahoppen ahoppen changed the title Filter index entries for deleted source files 🚥 #1175 Filter index entries for deleted source files Apr 24, 2024
@ahoppen ahoppen requested a review from bnbarham April 24, 2024 15:50
@ahoppen
Copy link
Collaborator Author

ahoppen commented Apr 24, 2024

@swift-ci Please test

@@ -257,15 +254,14 @@ extension SourceKitLSPServer {

let syntacticTests = try await languageService.syntacticDocumentTests(for: req.textDocument.uri)

if let index = workspace.index {
if let index = workspace.index(checkedFor: .imMemoryModifiedFiles(documentManager)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if let index = workspace.index(checkedFor: .imMemoryModifiedFiles(documentManager)) {
if let index = workspace.index(checkedFor: .inMemoryModifiedFiles(documentManager)) {

Weirdly this is the first time I noticed the misspelling (despite there being many before this).

This introduces an abstraction layer around `IndexStoreDB` (yes, I known another one …) that consults the underlying `IndexStoreDB` and then checks whether the entries are up-to-date with respect to some `IndexCheckLevel`. Requests can specify how picky they want to be about declaring results from the index out-of-date.

The default choice right now is to not include index entries from source files that have been deleted (because those are definitely from lingering unit files) but include results even if the source file has been modified after it was last indexed. We do include results from files that have been modified since they were last indexed because: When a file gets modified, it's likely that some of the line:column locations in it are still correct – eg. if only one line is modified and if lines are inserted/deleted all locations above are still correct. For locations that are out of date, showing stale results is one of the best ways of communicating to the user that the index is out-of-date and that they need to rebuild. We might want to reconsider this default when we have background indexing.

rdar://125230833
rdar://126622963
@ahoppen
Copy link
Collaborator Author

ahoppen commented May 1, 2024

@swift-ci Please test

@ahoppen
Copy link
Collaborator Author

ahoppen commented May 1, 2024

@swift-ci Please test Windows

1 similar comment
@ahoppen
Copy link
Collaborator Author

ahoppen commented May 1, 2024

@swift-ci Please test Windows

@ahoppen
Copy link
Collaborator Author

ahoppen commented May 2, 2024

@swift-ci Please test

@ahoppen ahoppen merged commit 47607d6 into apple:main May 2, 2024
3 checks passed
@ahoppen ahoppen deleted the check-index-up-to-date branch May 2, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants