[libcxx-commits] [PATCH] D98109: [libcxx] Make path::absolute return paths with separators in preferred form

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

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
   path cwd;
-  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.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98109