sammccall added a comment.
Thanks for looking at this Jan, I should have put you on the reviewers in the
first place...
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:53
+ }
+ StringRef Path = Buf->get()->getBuffer().trim();
+ if (Path.empty()) {
----------------
jkorous wrote:
> The `trim()` is probably safe - just wondering if that could cause issues in
> some weird case.
Only if the path ends with a space, AFAICS. I don't think it's worth worrying
about.
================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:65
+ if (llvm::sys::fs::real_path(Path, Resolved))
+ return Path; // On error;
+ return Resolved.str();
----------------
jkorous wrote:
> Just wondering why you decided to not somehow propagate the error - are there
> scenarios where the path is correct yet not resolvable on real FS?
By definition probably not *correct* cases, but this will be hit if
/usr/bin/clang is a dangling symlink, or xcrun lies to us, etc.
There's no error *propagation* because error recovery is here: even if we can't
work out whether clang is a symlink, the best thing to do is create a mangler
that assumes it isn't. (Or maybe better to create a mangler that ignores this
path entirely, but I'm not convinced).
There's no value in knowing about this failure at the site where the mangler is
created, or where the compile command is mangled, or where the file is opened -
we're always going to want to recover and forge ahead, and we have the best
information to recover here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71029/new/
https://reviews.llvm.org/D71029
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
