sammccall added inline comments.
================
Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:56
+// C++ and C Standard Library symbols are considered distinct: e.g. std::printf
+// and ::printf are not treated as the same symbol. The callers have to act
+// accordingly based on available LanguageOptions.
----------------
I'm not sure what "the callers have to act accordingly..." means - can you drop
it or elaborate?
================
Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:37
+ static llvm::Optional<Symbol> named(llvm::StringRef Scope,
+ llvm::StringRef Name);
+
----------------
kbobyrev wrote:
> kadircet wrote:
> > should scope have trailing `::` ?
> This is consistent with the current behavior; we can probably change it later.
Please document the behavior that's being added here.
(In clangd, "scope" fairly consistently includes "::" - I don't think that
convention exists here)
================
Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:66
+ Recognizer();
+ llvm::Optional<Symbol> operator()(const Decl *D);
+
----------------
kbobyrev wrote:
> kadircet wrote:
> > what about macros?
> For now, I'm just moving the code without adding any new capabilities. The
> only change is in `namespaceSymbols` (to break the dependency on clangd
> helpers) that Sam pointed out.
Please add a mention of macros to the docs.
================
Comment at: clang/lib/Tooling/Inclusions/StandardLibrary.cpp:119
+ return namespaceSymbols(Parent);
+ return NamespaceSymbols->lookup(D->getQualifiedNameAsString() + "::");
+ }();
----------------
Does this do the right thing for `std::__1::chrono` where `__1` is inline?
================
Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:61
+
+ namespace chrono {}
+
----------------
should there be symbols in this NS and a test for them?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119130/new/
https://reviews.llvm.org/D119130
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
