[PATCH] D114077: [clangd] Basic IncludeCleaner support for c/c++ standard library

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.

LG with a nit, thanks!



================
Comment at: clang-tools-extra/clangd/Headers.h:330
+  static inline clang::clangd::stdlib::Header getEmptyKey() {
+    return clang::clangd::stdlib::Header(-1);
+  }
----------------
sammccall wrote:
> kbobyrev wrote:
> > maybe `DenseMapInfo<unsigned>::getEmptyKey()` and `DenseMapInfo<unsigned>::getTombstoneKey()` just like above?
> empty/tombstone keys are reserved values, and we know what sensible reserved values are better than the traits for unsigned do. 
> 
> I can fix the code above if you like.
Okay, that makes sense! Yes, that would be great if you could fix the code above for consistency :)


================
Comment at: clang-tools-extra/clangd/IncludeCleaner.h:90
+/// FIXME: remove this hack once the implementation is good enough.
+void setIncludeCleanerAnalyzesStdlib(bool B);
+
----------------
sammccall wrote:
> kbobyrev wrote:
> > Not sure but: don't we want a config option instead? We can remove it just as easily since it's all "hidden" right now.
> I think we discussed this offline a bunch?
> 
> A config option is a bunch more plumbing, and we don't actually want to expose this option.
Oh, right, thanks for reminding!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114077/new/

https://reviews.llvm.org/D114077