[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

> When targeting Windows, the path separator used when targeting Windows depends on the build environment when Clang _itself_ is built. This leads to inconsistencies in Chrome builds where Clang running on non-Windows environments uses the forward slash (/) path separator while Clang running on Windows builds uses the backslash (\) path separator. To fix this, we make Clang use forward slashes whenever it is targeting Windows.

IMO, this problem formulation is a bit too narrow/specific - you'd have the same phenomenon if you'd cross compile things on Windows, targeting e.g. Linux. So I think the condition for normalizing paths would be if the build host OS is Windows, not based on the target OS. (I.e. in the current patch form, the normalization that is chosen, when targeting Windows from Unix, is a no-op anyway.)

Then secondly - if the source paths are relative paths, making them OS-agnostic in this way does make sense. But if they are absolute, they are pretty much by definition specific to the build host, and can't be expected to make sense on a different platform. So for such a case, there's less reason to try to normalize the paths. But as Windows path handling in most cases does tolerate paths with forward slashes, I guess it's ok to do anyway.

Overall, I think this is a sensible thing to do. I'm not entirely sure if an option for the behaviour is needed or not though - I guess that's the main question.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766