dexonsmith added a comment.

In D122766#3450298 <https://reviews.llvm.org/D122766#3450298>, @ayzhao wrote:

> So, the general consensus seems to be that we should use backslashes when 
> targeting Windows.
>
> I implemented using only backslashes for Windows; however, 
> clang/test/SemaCXX/destructor.cpp 
> <https://github.com/llvm/llvm-project/blob/main/clang/test/SemaCXX/destructor.cpp>
>  fails when running on Linux with the following error (among other errors, 
> but the one below is the most important).
>
>   ...
>   error: 'error' diagnostics seen but not expected:
>     Line 32: 
> '<REDACTED_PATH_TO_LLVM_CHECKOUT_WITH_BACKSLASHES>\\clang\\test\\SemaCXX\\destructor.cpp'
>  file not found
>   ...
>
> The reason for this is that the test has Clang target windows 
> <https://github.com/llvm/llvm-project/blob/77d2c815f50b20d18f1207e4f442e2cf8eb8cec0/clang/test/SemaCXX/destructor.cpp#L2>
>  and the test also has the statement #include __FILE__ 
> <https://github.com/llvm/llvm-project/blob/77d2c815f50b20d18f1207e4f442e2cf8eb8cec0/clang/test/SemaCXX/destructor.cpp#L32>.
>
> One way to fix this would be to have every macro that accepts a path 
> internally convert the path to the build environment's path format, but TBH 
> I'm not sure. What do you all think?

Wow, `#include __FILE__` is kind of amazing...

It sounds like you're suggesting changing the `#include` directive to start 
canonicalizing its argument. I'm not a fan of that idea; header search is hard 
enough to reason about.

I suggest, instead:

- Only canonicalize `__FILE__` for the target platform when there's a 
command-line flag that suggests it's okay. I suggest adding 
`-ffile-reproducible`, which could be implied by `-ffile-prefix-map` but also 
specified separately when `-ffile-prefix-map` isn't being used.
- When `__FILE__` is being canonicalized, it's okay to fail on `#include 
__FILE__` if the canonicalized path doesn't work as an argument to `#include` 
in the current build environment.

This would mean that `-ffile-reproducible` makes `#include __FILE__` 
non-portable, which would be fine as long as `#include __FILE__` isn't 
load-bearing for real life usecases. My guess is that it'd be okay?

What do others think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to