sammccall marked an inline comment as done. sammccall added inline comments.
================ Comment at: clang-tools-extra/clangd/CompileCommands.cpp:65 + if (llvm::sys::fs::real_path(Path, Resolved)) + return Path; // On error; + return Resolved.str(); ---------------- sammccall wrote: > 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. (Forgot to mention: I added a log here, since this shouldn't happen.) 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
