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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
