kadircet added a comment. Regarding the include mapping generator, I think it would've been better if we had some sort of list directly from libc++ (as this is now being part of clang rather than just clangd), but having the current symbol mapping available for other tools too is definitely a useful improvement and implementation details can change later on. I think we should have some "more public" documentation available around the limitations of current generator though, so that people are not surprised and aware of the caveats (like symbols might be missing, or in case of ambiguity they might be dropped, etc). Not sure where that belongs though, maybe header of the `.inc` file, or if we want it to be only used through the recognizer interface, maybe we can make the `inc` file "private" and document it there.
As for stdlib symbol/header/recognizer, I've got a couple questions around the functionality we are exposing: - (briefly mentioned above) should we just make raw symbol mappings an implementation detail of stdlib recognizer and have applications depend on it? - do we want headers/symbols to be created outside of the recognizer? ================ Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:15 +public: + static llvm::Optional<Header> named(llvm::StringRef Name); + ---------------- can you clarify if `Name` can have quotes/angles ? ================ Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:37 + static llvm::Optional<Symbol> named(llvm::StringRef Scope, + llvm::StringRef Name); + ---------------- should scope have trailing `::` ? ================ Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:46 + Header header() const; + // Some symbols may be provided my multiple headers. + llvm::SmallVector<Header> headers() const; ---------------- s/my/by ================ Comment at: clang/include/clang/Tooling/Inclusions/StandardLibrary.h:66 + Recognizer(); + llvm::Optional<Symbol> operator()(const Decl *D); + ---------------- what about macros? 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
