[libcxx-commits] [PATCH] D98106: [libcxx] [test] Fix lexically_normal and lexically_relative_and_proximate for windows
Martin Storsjö via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Mar 6 09:19:46 PST 2021
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
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.
Sure, I can do a separate change to improve the consistency that way.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits