kbobyrev added a comment.
Mostly LG, few nits.
================
Comment at: clang-tools-extra/clangd/Headers.h:32
namespace clang {
+class NamespaceDecl;
namespace clangd {
----------------
Do we need a forward decl here?
================
Comment at: clang-tools-extra/clangd/Headers.h:42
+public:
+ static llvm::Optional<Header> named(llvm::StringRef);
+
----------------
nit: maybe mention the parameter name? it seems redundant but consistent with
what we have around here.
================
Comment at: clang-tools-extra/clangd/Headers.h:72
+ Header header() const;
+ llvm::SmallVector<Header> headers() const;
+
----------------
What's the difference between header() and headers()? Without looking at the
code below, I presume this is for symbols that could be defined in multiple
headers?
================
Comment at: clang-tools-extra/clangd/Headers.h:330
+ static inline clang::clangd::stdlib::Header getEmptyKey() {
+ return clang::clangd::stdlib::Header(-1);
+ }
----------------
maybe `DenseMapInfo<unsigned>::getEmptyKey()` and
`DenseMapInfo<unsigned>::getTombstoneKey()` just like above?
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:116
+ Result.User.insert(Redecl->getLocation());
+ if (auto SS = StdRecognizer(D))
+ Result.Stdlib.insert(*SS);
----------------
Maybe pull this upstairs and use early return instead? It's either user code or
Stdlib, so maybe add exclusively to one of the sets.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.h:90
+/// FIXME: remove this hack once the implementation is good enough.
+void setIncludeCleanerAnalyzesStdlib(bool B);
+
----------------
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.
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
