kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1220
CDB->setCompileCommand(File, std::move(New));
ModifiedFiles.insert(File);
}
----------------
sammccall wrote:
> kadircet wrote:
> > nit: maybe just set `ReparseAllFiles` in here too, and change the condition
> > below to only `return ReparseAllFiles`
> Is the idea here that triggering spurious reparses is cheap enough?
> Or that if it's cheap enough for our "future-facing" extension, we should be
> able to stop optimizing it for the existing one?
none :( sorry i misread this and thought the code below was doing
`ModifiedFiles.size() != 0` (that's way the comment was a nit)
OTOH, as you noted spurious reparses are cheap(still some IO and a task in the
queue). We expect the usage of updating compile commands to migrate towards
configs in the future, so in theory we won't be using that optimization most of
the time.
Even though we've got enough info to prevent spurious reparses here, we might
not in the future (e.g. reparses resulting from a config change). So I would
drop the optimization(the whole Old-New comparison logic) to increase
readability.
but definitely not something we should do now (probably ever), so feel free to
ignore.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83822/new/
https://reviews.llvm.org/D83822
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
