[libcxx-commits] [PATCH] D98106: [libcxx] [test] Fix lexically_normal and lexically_relative_and_proximate for windows

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

LGTM!



================
Comment at: libcxx/test/std/input.output/filesystems/class.path/path.member/path.gen/lexically_relative_and_proximate.pass.cpp:88
     const fs::path proximate_expected = output.native().empty() ? p
         : output;
     if (!PathEq(proximate_expected, proximate_output))
----------------
Potential drive-by clarity improvement: Here we construct `proximate_expected` from the old value of `output`, rather than from `expect`. This works, because by the time we reach this line, it is guaranteed that `PathEq(output, expect)`. Still, IMHO, this code would be less confusing if you replaced the identifier `output` on this line with the identifier `expect`. Plus, fix the argument order to `PathEq` so that it's parallel to the order on line 81.

    const fs::path proximate_expect = expect.empty() ? p : expect;
    if (!PathEq(proximate_output, proximate_expect))
      ~~~

Not a blocker, but I think it'd be nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98106