[PATCH] D112288: Support: Expose sys::path::system_style()

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: llvm/include/llvm/Support/Path.h:37
+  return Style::posix;
+#endif
+}
----------------
mstorsjo wrote:
> FYI, I have a patchset for extending the handling of windows path styles into two styles, windows-with-backslashes and windows-with-forward-slashes; the first patch is up for review at https://reviews.llvm.org/D111879, but later in the series, I would be extending `system_style()` to return either of the windows styles in various cases.
> 
> My current patch that does that (that I haven't sent for review yet) is at https://github.com/mstorsjo/llvm-project/commit/c609500ef26145dcca7d853c9ceabbe1402fc4f2, where I just add another `#ifdef __MINGW32__` here, which would work fine with constexpr.
> 
> But I'm considering other alternatives for enabling that mode, where having the function as constexpr in a header isn't entirely ideal. One alternative would be to make a cmake option for choosing the default preferred path form - and having a generated `config.h` included in a pretty widely included header wouldn't be good. An even wilder option (that I don't think I'd propose though) would be to make it runtime settable (as a global variable/setting).
> 
> I'd like if @rnk could chime in here - what amount of flexibility/settability do you forsee for that feature - does that conflict with having it constexpr in a header?
> 
> Then on the other hand, even if it is constexpr in a header right now, I guess it should be doable to change it to a non-constexpr function later if needed, as long as it isn't used in a constexpr context in the callers. It won't generate quite as optimal code in the callers, but I think the difference would be negligible.
Also, in D111879 I'm adding a `bool is_style_windows(Style style)` helper inside Path.cpp, but I guess it could be exposed publicly too. And with that in mind, another way of keeping the constexpr-ness, but much less elegant though, is to add a separate `bool system_style_is_windows()` - because regardless which slash preference is chosen on windows, we still know for sure at compile time that it's going to be either of the windows path styles.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112288