kadircet marked 2 inline comments as done.
kadircet added a comment.
as discussed offline, this patch stores the filepath provided by the client
(e.g. vscode) which is usually the filepath seen by the user, inside the
ParsedAST and later on uses that when a hint is needed for translating
filepaths coming from sourcemanager into uris.
this gets rid of the extra canonicalization needed for the main file path
(which could fail when we don't have a current working directory set, i.e. in
tests) and also does always the "right" thing as all the files should be
relative to the workspace of the main file path seen by the user, not the
version clangd derives somehow.
================
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,
----------------
ilya-biryukov wrote:
> 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.
this is a private constructor and public interface also receives a stringref
and none of the production callers actually have an extra copy of the filename
lying around. so i'd rather keep it as is.
================
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; }
----------------
ilya-biryukov wrote:
> 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".
i'd rather not complicate the wording here. i think mentioning the fact that
this is the filepath as provided by the caller is enough. being used as a hint
for canonicalization of other paths is something specific to the application
(and is a result of being the path provided by caller anyway.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130690/new/
https://reviews.llvm.org/D130690
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
