[PATCH] D54077: [clangd] Implemented DraftFileSystem

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 6 00:07:30 PST 2018


sammccall added a comment.

I don't think either point 1 or 2 above have been addressed, and so I'm not comfortable moving forward with this change.

> I think it would be reasonable to say that a large portion of C++ users are used to the behavior that this patch introduces.

I agree, the new behavior is reasonable. The old behavior is also reasonable. This is the basis for point 1 above.

> Current behavior may confuse new users, since, other IDEs mostly (all?) shows diagnostics for edited files instead of saved one. And it's unexpected that headers have 2 states - visible and saved (which cannot be viewed in IDE at all).

Maybe part of the issue is culture clash between "IDEs" and "editors" - the distinction is somewhat artificial, but I think the current behavior in editors is somewhat traditional, and certainly the two-states is what editor users expect (the saved state is what you get when you run the compiler).

> Looks like performance will be same like usage of 'auto save after delay' feature in editor, if we make debounce delay configurable, what do you think?

I think there will be substantial performance loss *and* additional complexity from this mode once we start updating diagnostics on file change. (Point 2 above).
Additionally, if this needs to be configurable, that adds further complexity.

In https://reviews.llvm.org/D54077#1287301, @simark wrote:

> Of course, that is dependent of having an efficient cancelling mechanism.  Even with some debouncing, an update to a header file can trigger the re-processing of many TUs, so if I do another edit to the same header shortly after, all the queued jobs from the first edit should be dropped.  But I think we already have that, don't we?


Not for preambles (as we don't yet even update diagnostics on file events), and that case is significantly more complicated than main-files:

- there are multiple TUs to consider as you say
- preambles can be 100x more expensive than mainfiles
- this affects background files, so is "usually" unimportant work
- this happens while latency-sensitive operations like code-complete happen in the header file itself


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54077





More information about the cfe-commits mailing list