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

[Vinicius Tinti via Phabricator via llvm-commits]

tinti added inline comments.


================
Comment at: llvm/include/llvm/Support/Path.h:454
 ///
+/// C++17 rules are:
+/// 1) POSIX style paths with nonempty root directory are absolute.
----------------
jhenderson wrote:
> It might be worth adding the suggested quote, taken from https://en.cppreference.com/w/cpp/filesystem/path/is_absrel prior to the list.
Ok.

Useful:

```
Notes: The path "/" is absolute on a POSIX OS, but is relative on Windows.
```


================
Comment at: llvm/lib/Support/Path.cpp:690
+
+  // Handle '/' which should be true regardless Windows or POSIX.
+  // Handle '\\' on Windows.
----------------
jhenderson wrote:
> jhenderson wrote:
> > 
> You can delete "on GNU", since it's implied (we're in `is_absolute_gnu` already).
Ok. I agree.

I tried to avoid make affirmations that would not be true depending on C++17 or GNU. In such cases I tried to add context.


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

https://reviews.llvm.org/D87667