[libcxx-commits] [PATCH] D91178: [22/N] [libcxx] Implement append and operator/ properly for windows

[Adrian McCarthy via Phabricator via libcxx-commits]

amccarth requested changes to this revision.
amccarth added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/filesystem:1033
+    __pn_ += __p.__pn_.substr(__p_root_name_size);
+    return *this;
+  }
----------------
The above looks like a literal translation of the standard, so I'd expected it to be platform agnostic.  What makes it Windows-specific?


================
Comment at: libcxx/test/std/input.output/filesystems/class.path/path.member/path.append.pass.cpp:75
+      , {S("foo"),     S("C:/bar"),   S("C:/bar")}
+      , {S("foo"),     S("C:"),       S("C:")}
+
----------------
I expected 'C:foo`.  Using PathAppend, I get `C:\` (absolute, not relative).

It's interesting (to me) that the `<filesystem>` rules don't give either of those answers.

[This is just an observation.  Nothing to do here.]


================
Comment at: libcxx/test/std/input.output/filesystems/class.path/path.member/path.append.pass.cpp:97
+      , {S("//host/"), S("foo"),      S("//host/foo")}
+      , {S("//host"),  S(""),         S("//host/")}
+
----------------
The three UNC examples above exactly match those in the `#ifdef _WIN32` branch, so perhaps they should be hoisted above the ifdef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91178