[PATCH] D112787: [Support] Allow configuring the preferred type of slashes on Windows

Thu Nov 4 14:48:39 PDT 2021

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM modulo the comment request.

================
Comment at: llvm/CMakeLists.txt:357
+set(WINDOWS_PREFER_FORWARD_SLASH_DEFAULT OFF)
+if (MINGW)
+  set(WINDOWS_PREFER_FORWARD_SLASH_DEFAULT ON)
----------------
mstorsjo wrote:
> aaron.ballman wrote:
> > mstorsjo wrote:
> > > aaron.ballman wrote:
> > > > Should this also happen for cygwin?
> > > I think cygwin shouldn't matter here - cygwin mostly presents itself as a unix; it doesn't define `_WIN32` when compiling etc, so on cygwin you'd get the posix path style.
> > Ooohh! So because `_WIN32` isn't defined, we will prefer forward slashes already? (What confused me is this looks like it prefers backslash on *all* Windows targets except mingw and it seems like we want the same for cygwin.)
> Yes - but not Windows-style paths with forward slashes - it gets plain posix paths only.
> 
> As cygwin doesn't define `_WIN32` and thus doesn't present itself as Windows at all, LLVM built in cygwin thinks it's executing on a regular unix, so `path::Style::native` does evaluate to `path::Style::posix`, and it's executing code from llvm/lib/Support/Unix instead of llvm/lib/Support/Windows, etc.
> 
> So to avoid implying that it has an effect, I wouldn't add it to the if condition here, but I guess it would warrant a comment. Maybe something along these lines:
> 
> ```
> if (MINGW)
>   # Cygwin doesn't identify itself as Windows, and thus gets path::Style::posix as native path style,
>   # regardless of what this is set to.
>   set(WINDOWS_PREFER_FORWARD_SLASH_DEFAULT ON)
> endif()
> ```
Thank you for the explanation! I think that comment would help others reading the CMake without that context in mind.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112787