sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/Headers.h:32
namespace clang {
+class NamespaceDecl;
namespace clangd {
----------------
kbobyrev wrote:
> Do we need a forward decl here?
Decl/NamespaceDecl are needed for the interface of the stdlib symbol
recognizer, but we don't need to depend on any of the details of AST here.
Splitting the stdlib stuff into its own header seems possible if you'd prefer
that?
================
Comment at: clang-tools-extra/clangd/Headers.h:330
+ static inline clang::clangd::stdlib::Header getEmptyKey() {
+ return clang::clangd::stdlib::Header(-1);
+ }
----------------
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.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.h:90
+/// FIXME: remove this hack once the implementation is good enough.
+void setIncludeCleanerAnalyzesStdlib(bool B);
+
----------------
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.
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
