Page MenuHomePhabricator

[clangd] Allow updates to be canceled after compile flags retrieval
Needs ReviewPublic

Authored by kadircet on Aug 10 2022, 7:35 AM.

Details

Reviewers
ilya-biryukov
Summary

Retrieving compile flags might invalidate the environment of an update
request (e.g. generating build artifacts that wasn't captured at the time of
AddDocument notification). This enables clients with such behavior to optimize
away these intermediate AST builds.

Diff Detail

Event Timeline

kadircet created this revision.Aug 10 2022, 7:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2022, 7:35 AM
kadircet requested review of this revision.Aug 10 2022, 7:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2022, 7:35 AM

Could you please add a test with a compilation database that models the blocking behavior, attempts cancellation and ensures no diagnostics are produced?
I also suggest adding a callback similar to Callbacks.onFailedAST, which could be used for testing this behavior.
The test should also check that subsequent updates and reads get resolved properly.

We don't have an actual implementation for the client code that relies on this in-tree, but the unit test will help to ensure we don't break this contract.

adamcz added a subscriber: adamcz.Aug 11 2022, 5:25 AM
adamcz added inline comments.
clang-tools-extra/clangd/TUScheduler.cpp
898

I'd add something like "due to cancellation". I know you can tell that from the line number, but at first glance someone might think it's due to lack of changes or something like that.