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

[Vinicius Tinti via Phabricator via llvm-commits]

tinti marked an inline comment as done.
tinti added a comment.

I have done several rewords.

I think it is better now. I hope I haven't missed more suggestions.



================
Comment at: llvm/include/llvm/Support/Path.h:466
+///
+/// LLVM \ref is_absolute rules are different.
+///
----------------
jhenderson wrote:
> jhenderson wrote:
> > It might be worth adding to the `is_absolute` comment expanding the LLVM rules, so that people don't read this and go "okay, what are the differences?"
> As mentioned in my previous comment, I think you should move the comment to the `is_absolute` function. I'd also remove reference to `LLVM`, since this applies to the standard C++17 `is_absolute` function too, I believe.
I missed the move part.


================
Comment at: llvm/unittests/Support/Path.cpp:90
+  // Test tuple <Path, ExpectedPosixValue, ExpectedWindowsValue>.
+  const SmallVector<std::tuple<StringRef, bool, bool>, 13> Paths = {
+      {"", false, false},  {"/", true, true},      {"/foo", true, true},
----------------
jhenderson wrote:
> Why are you still using `SmallVector` here? I suggested an array before, and I don't see any benefit to the `SmallVector` (we don't need vector operations for this test).
My bad. I missed this one too.


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

https://reviews.llvm.org/D87667