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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added reviewers: Bigcheese, jansvoboda11, arphaman.
dexonsmith added a comment.

I missed this before, and it's an interesting problem.

I have a few questions:

- I'm not clear on the expected interaction between the current working directory of the compiler and `-fsource-dir`. What happens if they're different?
- I'm curious if you've considered other solutions that tie the source directory more closely to the current working directory, such as a flag `-frelative-to-working-dir`, which makes things relative to the compiler's working directory. Is that too restrictive?
- Have you looked at the similar "MakeRelative" logic in `ASTWriter.cpp`? Can this logic be shared somehow?
- What's the expected behaviour when building modules implicitly with `-fmodules`? What value (if any) should they get for `-fsource-dir`?
- How should this affect the dependency output (e.g., `clang-scan-deps` and `.d` files), if at all?
- Have you considered having `-fsource-dir` add entries to the prefix maps, rather than duplicating logic?

I am a bit concerned about mixing the concepts of "relative base path" with "source directory", since the latter sounds to me like where pristine sources live and that's not always the right relative base path.

For example, consider a layout like this:

  /working/dir/
               sources/
                       lib/a/a.c
                       include/a/a.h
                       include/a/a.td
               build/
                     include/lib/a/a.inc
                     lib/a/a.o
               toolchain/usr/
                             bin/clang
                             lib/clang/11.0/include/stdint.h
               sdk/usr/include/
                               stdint.h
                               c++/v1/
                                      vector
                                      stdint.h

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.

WDYT?



================
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1545-1547
+      StringRef SourceDir = PPOpts->SourceDir;
+      if (!SourceDir.empty() && FN.startswith(SourceDir))
+        llvm::sys::path::make_relative(FN, SourceDir, FN);
----------------
What does this do with `-fsource-dir /a/b` for the file `/a/b.c`? (It doesn't seem quite right for it to turn it into `.c` or `../b.c`...)


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

https://reviews.llvm.org/D87928