kadircet added a comment.
(not sure if you were looking for comments yet, but i was just passing by and
it was a small-ish patch, so couldn't resist :D)
================
Comment at: clang-tools-extra/clangd/index/Background.cpp:449
+ // TODO(kuhnel): is the a better place to store this file?
+ // TODO(kuhnel): do we need a file at all, can we just pass a string to the
+ // indexer?
----------------
In theory BackgroundIndex only reads headers from the FS, so we can provide an
in-memory buffer as the main file itself. `prepareCompilerInstance` in
`Compiler.h` (and used by `BackgroundIndex::index` does that exactly).
It might be better to just have a separate endpoint in BackgroundIndex (as
`indexSTLHeaders`) that is invoked once at construction time that enqueues
indexing of this phantom file.
WDYT?
================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:527
+opt<bool> IndexSTL{
+ "index-stl",
----------------
do we really need to hide this behind a flag ? it sounds like a quite useful
feature to me with minimal risk of regressions and unlikely to make anyone
upset. in the end we are not doing anything heuristically and just throwing
clang on some STL headers, that'll probably be included within the TUs
eventually.
there's always the risk of crashing while parsing some STL headers, but i don't
think it is any different than user just including the header manually.
the biggest problem i can see is people using custom STL headers, but hopefully
compile flags interpolation logic should be able to infer the relevant location
for those. and in the cases it fails we either index the default STL and
suggest people some symbols their implementation might lack, or fail to find
STL at all and print some logs for the missing includes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94293/new/
https://reviews.llvm.org/D94293
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
