[libcxx-commits] [PATCH] D89943: [libcxx] [test] Fix path.decompose for windows

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM.



================
Comment at: libcxx/test/std/input.output/filesystems/class.path/path.member/path.decompose/path.decompose.pass.cpp:146
 
+#ifdef _WIN32
+    if (!p.has_root_name()) {
+      assert(p.is_absolute() == false);
+    } else {
+      std::string root_name = p.root_name().string();
+      assert(root_name.length() >= 2);
+      if (root_name[1] == ':') {
+        // Drive letter, absolute if has a root directory
+        assert(p.is_absolute() == p.has_root_directory());
+      } else {
+        // Possibly a server path
+        // Convert to one separator style, for simplicity
+        std::replace(root_name.begin(), root_name.end(), '\\', '/');
+        if (root_name[0] == '/' && root_name[1] == '/')
+          assert(p.is_absolute() == true);
+        else
+          assert(p.is_absolute() == false);
+      }
+    }
+#else
     assert(p.is_absolute() == p.has_root_directory());
     assert(p.is_relative() != p.is_absolute());
----------------
mstorsjo wrote:
> curdeius wrote:
> > On a second thought... Instead of all this stuff, shouldn't we just add `bool is_absolute` to `PathDecomposeTestcase`?
> I guess we could, but that would require both touching all lines in the table to add another field. That's maybe doable, but worse, we'd need to have that field vary in more cases. E.g. `/foo/bar` is an absolute path on posix, but not on windows (as it lacks a drive name, the path is not considered absolute). So it would essentially require ifdeffing/duplicating the whole table instead of just some sections of it.
Oh, that's a pity. I hadn't thought about the cases outside of the offers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89943