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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
