[libcxx-commits] [PATCH] D98109: [libcxx] Make path::absolute return paths with separators in preferred form
Arthur O'Dwyer via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Mar 6 08:45:02 PST 2021
Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.
Comment at: libcxx/src/filesystem/operations.cpp:701
- return __do_absolute(p, &cwd, ec);
+ return __do_absolute(p, &cwd, ec).make_preferred();
Your friendly neighborhood `return x` expert says: Yikes! Remember that `make_preferred()` modifies the string in place (which is OK, and I checked that it's efficient) but then returns //an lvalue reference to the path//, not a copy of the path!
So the old code was RVO'ing the prvalue result of `__do_absolute`. The new code, on Windows //and even on POSIX where `make_preferred` is a no-op//, is doing a //copy-construct// from the lvalue result of `make_preferred()` into the return slot.
Please, at a minimum, change to use `return cwd;`.
If you would like to do larger surgery on `__do_absolute`, to make it more efficient, that would be awesome and I'd definitely take a look at it. (Right now I don't even understand why it takes `cwd` at all; it seems like we do that so that `__canonical` can throw a `filesystem_error` with `path2=cwd`, but that's [not even what cppreference says should happen](https://en.cppreference.com/w/cpp/filesystem/canonical) so I don't know why we do that. Some digging will be required by whoever takes this on.)
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits