[PATCH] D87667: [Support/Path] Add path::is_gnu_style_absolute

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

> On Windows, paths starting with drive letter pattern are absolute.

This isn't a difference is it? That's also the behaviour for `is_absolute`, I believe?



================
Comment at: llvm/include/llvm/Support/Path.h:465-468
+/// GNU style absolute define that:
+/// 1) Paths starting with '/' are always absolute;
+/// 2) On Windows, paths starting with '\\' are absolute;
+/// 3) On Windows, paths starting with drive letter pattern are absolute.
----------------



================
Comment at: llvm/include/llvm/Support/Path.h:470
+///
+/// @param path Input path.
+/// @result True if the path is GNU style absolute, false if it is not.
----------------
You need to explain the purpose of the `style` parameter too. I don't know why this is missing from the other doc comments around here.


================
Comment at: llvm/lib/Support/Path.cpp:690
 
+bool is_gnu_style_absolute(const Twine &Path, Style Style) {
+  SmallString<128> PathStorage;
----------------
I'd put this function adjacent to `is_absolute`, like it is in the header.

Also, I think you can use the same naming style for variables as in `is_absolute` (i.e. lowerCameCase).

Rather than implement this whole function from scratch, maybe you could just do the bit that's different from `is_absolute` first, and then just call `is_absolute`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87667