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

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
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))
----------------
Quuxplusone wrote:
> 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.
Sure, I can do a separate change to improve the consistency that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98106