[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 28 07:13:16 PDT 2022


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



More information about the cfe-commits mailing list