ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM and a few nits. Could you please add a summary of our offline discussion
here.
================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:763
-ParsedAST::ParsedAST(llvm::StringRef Version,
+ParsedAST::ParsedAST(PathRef TUPath, llvm::StringRef Version,
std::shared_ptr<const PreambleData> Preamble,
----------------
NIT: use `Path`.
this allows the callers to avoid an extra allocation, e.g. if they already
allocated a `std::string` and they can `std::move` into the constructor.
================
Comment at: clang-tools-extra/clangd/ParsedAST.h:112
+ /// Returns the path passed by the caller when building this AST.
+ PathRef tuPath() const { return TUPath; }
----------------
NIT: could you add a comment how this is used, e.g. `this path is used as a
hint when canonicalize the path`.
Also useful to document that we expect this to be absolute/"canonical".
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130690/new/
https://reviews.llvm.org/D130690
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
