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

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

Reply via email to