kadircet added inline comments.
================ Comment at: clang-tools-extra/clangd/CompileCommands.cpp:141 + // Let's hope it's not a symlink. + if (llvm::any_of(Driver, + [](char C) { return llvm::sys::path::is_separator(C); })) ---------------- sammccall wrote: > kadircet wrote: > > I believe it would be clearer if you put it below, into `else if > > (ClangPath)` part. > > > > i.e. > > ``` > > if (Absolute ...) > > .. > > // If we couldn't find program and driver is just a filename, use clang dir > > // FIXME: Note that Driver can still be relative to workdir even if it > > doesn't have any path separators. > > // We should pass WD into here and try to make Driver absolute. > > else if(ClangPath && !hasPathSeparators(Driver)) > > ... > > ``` > This would rely on the undocumented behaviour of findProgramByName with > workdir-relative paths, which I think would be nice to avoid (for clarity to > the reader if nothing else). > Doing (what looks like) a PATH search for a workdir-relative path is just > conceptually wrong. makes sense, i think we should land this soon-ish to stop the bleeding. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82011/new/ https://reviews.llvm.org/D82011 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits