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

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius added inline comments.


================
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());
----------------
On a second thought... Instead of all this stuff, shouldn't we just add `bool is_absolute` to `PathDecomposeTestcase`?


================
Comment at: libcxx/test/std/input.output/filesystems/class.path/path.member/path.decompose/path.decompose.pass.cpp:184
+        std::replace(root_name.begin(), root_name.end(), '\\', '/');
+        if (root_name[0] == '/' && root_name[1] == '/')
+          assert(p.is_absolute() == true);
----------------
mstorsjo wrote:
> curdeius wrote:
> > root_name here can possibly have less than 2 characters. You should probably assert that if it's non-empty, then it should have size() >= 2.
> Good point, will do that.
> 
> Technically, we're not facing unknown inputs here, we only ever get things based on inputs in the table above (and whatever a more or less broken implementation returns based on it), but it's certainly good to verify this in any case.
> Technically, we're not facing unknown inputs here, we only ever get things based on inputs in the table above (and whatever a more or less broken implementation returns based on it), but it's certainly good to verify this in any case.
100% agree.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89943