[PATCH] D54077: [clangd] Implemented DraftFileSystem
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 5 04:43:44 PST 2018
sammccall added subscribers: ilya-biryukov, LutsenkoDanil.
sammccall added a comment.
Disclaimer: I'm on a train with family today, and haven't actually read the
patch...
So I have concerns :-)
1. There's the usual concern that the current behavior is reasonable and
people like it, so adding a second reasonable behavior provides a small
amount of value to the userbase as a whole (because 99%+ will use the
default). If the benefit is small, the comparison to support cost may be
unfavorable.
2. This needs to invalidate preambles more often, which brings both
performance and complexity questions. E.g. we will at some point invalidate
preambles and regenerate diagnostics based on file writes. After this
change, we'll be obligated to do so for edits too. (Remember, LSP has no
concept of "the foreground file"). Invalidating a preamble is expensive,
and edits come rapidly and may invalidate multiple TUs. The current
behavior seems more compatible with being both simple and fast.
So mostly I'd like to be convinced that this is important (or that it's
simple, but that seems unlikely at first glance).
Cheers, Sam
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54077
More information about the cfe-commits
mailing list