[libcxx-commits] [PATCH] D89945: [libcxx] [test] Use path::operator== instead of PathEq in the path.append test

Martin Storsjö via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Feb 25 11:03:35 PST 2021


mstorsjo added inline comments.


================
Comment at: libcxx/test/std/input.output/filesystems/class.path/path.member/path.append.pass.cpp:112
     }
-    assert(PathEq(LHS, E));
+    assert(LHS == E);
   }
----------------
ldionne wrote:
> I'm not sure I understand, can you give me an example of why this fails on Windows with `PathEq`?
`PathEq` checks that the strings are literally identical, while `path::operator==` checks that the paths mean the same (i.e. tolerating both slash forms on windows).

When we test appending `p2` to `p1` on windows, we get the path `p1\p2`, but the test reference is `p1/p2`.

We can't just run `make_preferred()` on the reference and do a literal comparison either: If the original path is `p1/p2` and we append `p3`, we get `p1/p2\p3`.

So this weakens the test a bit (it doesn't distinguish between `p1/p2` and `p1//p2` iirc either), but is a simple change to make. The alternative would be to ifdef the whole table and add duplicate references (or to add `ref_posix`, `ref_windows` in each table entry). Thinking of it now (3 months later), the last option actually sounds pretty good although it makes the patch messier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89945



More information about the libcxx-commits mailing list