[PATCH] D111879: [Support] Add a new path style for Windows with forward slashes

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: llvm/lib/Support/Path.cpp:608
 StringRef get_separator(Style style) {
-  if (is_style_windows(style))
+  if (real_style(style) == Style::windows)
     return "\\";
----------------
aaron.ballman wrote:
> Up on line 48, we use `is_style_windows()` instead of calling `real_style()`, but here we've flipped the edits. Is there a reason for the distinction?
Yes - on line 48 in `separators`, we want to return all accepted forms of separators - in both windows path styles, we tolerate both kinds of separators on input, i.e. we want to check if the style is any windows style. Here in `get_separator` (and similarly in `preferred_separator`) we return the one that we want to use when we concatenate/produce new paths (i.e. we want to check strictly if `real_style(style)` evaluates to `Style::windows_backslash` (`Style::windows` is an backwards compat alias for it).

(Changing existing occurrances of `Style::windows` into `Style::windows_backslash` for clarity could be done as a separate commit later.)

The calls to `is_style_windows()` were added recently in a refactoring (changing most cases of `real_style() == windows` into `is_style_windows()` in D112289), but those particular functions accidentally got changed the wrong way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111879