keith added inline comments.
================ Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1334 + llvm::SmallString<256> Path(Filename); + llvm::sys::fs::make_absolute(Path); + llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true); ---------------- rnk wrote: > keith wrote: > > rnk wrote: > > > Please only make the path absolute if nothing in the prefix map matches. > > > Otherwise, the user must embed the CWD into the prefix map, which is > > > needlessly difficult for the build system. I believe it is also > > > consistent with the way that the debug info prefix map works. It appears > > > to operate on the possibly relative source paths received on the command > > > line (-I...). > > Are you suggesting that I try to remap them relatively, and if that fails, > > absolutize it and attempt the remapping again? Or don't absolutize them at > > all anymore? > I'd prefer to do the remapping once without any absolutization, and if that > fails, preserve the behavior of absolutizing. > > It would be my preference to not absolutize paths at all, since that is what > is done for debug info. However, @vsk implemented things this way, and I do > understand that this is convenient default behavior for most users: the paths > in the coverage are valid anywhere on their system. So, changing this > behavior is out of scope for this patch. I'm not sure how this changed would work for our use case. With bazel the usage of this works something like: 1. Relative paths are passed to the compiler `clang ... foo.c` 2. We would normally do `-fprofile-prefix-map=$PWD=.` to remap them I think if we made this change, we would either have to: 1. Make the paths we pass absolute, which we couldn't do for reproducibility 2. Add some known prefix to the paths, like `.`, so we could `-fprofile-prefix-map=.=.` just to avoid this absolutizing codepath So I think without actually removing the absolutizing behavior at the same time, this wouldn't work as we'd hope. Let me know if I'm mis-understanding your suggestion! If we did what I said above, I think it would work since relative path remapping would fail, but then prefix mapping the absolute path would work. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83154/new/ https://reviews.llvm.org/D83154 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
