kuhnel added a comment.
Yes, I was looking for feedback on this one as I wasn't happy with my design.
================
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?
----------------
kadircet wrote:
> 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?
Agree to both. I'll take a look.
================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:527
+opt<bool> IndexSTL{
+ "index-stl",
----------------
kadircet wrote:
> 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.
Back when I was developing embedded software, we couldn't use the STL for
various reasons (object size, no exceptions, no dynamic memory, ...) and had
our own, proprietary base library. In such a scenario it would be annoying if
clangd would be proposing things I could not use in the project.
So we should have a way for users to disable this. I'm fine if we switch it on
by default and offer a `--disable-stl-index` flag instead.
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
