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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added subscribers: rnk, mstorsjo.
mstorsjo added inline comments.


================
Comment at: llvm/include/llvm/Support/Path.h:37
+  return Style::posix;
+#endif
+}
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112288