[PATCH] D87928: Provide -fsource-dir flag in Clang

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 17 15:05:38 PST 2020


dexonsmith added a comment.

In D87928#2396448 <https://reviews.llvm.org/D87928#2396448>, @phosek wrote:

> `-fsource-dir` is only used for coverage mapping and macros.

Oh, interesting; it would be nice to consistently make all paths relative paths.

> Here `out/default` is the working directory so if we used `-frelative-to-working-dir`, we would end up with paths like `../../sources/lib/a/a.c` in the coverage mapping or macro expansions like `__FILE__`. While this may be acceptable for macro expansions, it's definitely undesirable in coverage mapping.
>
> I can think of some potential solutions. We use could use `-fprofile-prefix-map=../../=` to strip the `../../` prefix.

This solution makes sense to me.

>> - What's the expected behaviour when building modules implicitly with `-fmodules`? What value (if any) should they get for `-fsource-dir`?
>
> Right now this flag doesn't apply to modules at all so there's no effect.

I'm not quite following this. I don't see logic to avoid this flag applying to modules, although I may be missing something. I believe modules built explicitly via the command-line will happily accept and apply this option, and, similarly, I think modules built implicitly by cloning a `CompilerInvocation` will pick up the same value as the importing `CompilerInstance`.

>> It seems cleaner to have everything relative to `/working/dir`, not `/working/dir/sources`, since you can name all compiler inputs and outputs with the former. But the latter is conceptually the source directory.
>
> Yes, I agree. Ultimately it's up to the user to decide what output they expect. I guess this is one advantage of using the `-frelative-to-working-dir` approach where it's unambiguous and user can use other flags to do additional rewriting if needed.

If we want to handle it consistently everywhere, I wonder if we'd want a feature in the `FileManager` to do this, potentially handling many other cases. E.g., instead of canonicalizing to an absolute path, it would canonicalize to a path relative to working directory. Then callers just need to worry about the prefix map. I'm not sure though.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87928/new/

https://reviews.llvm.org/D87928



More information about the cfe-commits mailing list