[libcxx-commits] [PATCH] D91178: [22/N] [libcxx] Implement append and operator/ properly for windows

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Feb 17 12:39:21 PST 2021

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.

Comment at: libcxx/include/filesystem:1079
     using _CVT = _PathCVT<_SourceChar<_Source> >;
     if (__source_is_absolute(_Traits::__first_or_null(__src)))
If we end up keeping those functions separate in the Posix and non-Posix cases, we could get rid of `static bool __source_is_absolute(_ECharT)` above by instead doing:

bool __source_is_absolute = __is_separator(_Traits::__first_or_null(__src));
if (__source_is_absolute)
  // ...

I feel like having a separate function for `__source_is_absolute()` isn't worth the trouble anymore if we need to add `#ifdefs` around it & al.

Comment at: libcxx/include/filesystem:1033
+    __pn_ += __p.__pn_.substr(__p_root_name_size);
+    return *this;
+  }
mstorsjo wrote:
> amccarth wrote:
> > The above looks like a literal translation of the standard, so I'd expected it to be platform agnostic.  What makes it Windows-specific?
> It's fairly close to the reference yes. It's not very windows specific in itself, but libc++'s current posix-only implementation is much more straightforward due to having less complicated rules (where there's no root name concept at all).
Would this implementation work on Posix as well? Looking at the Posix implementation, it doesn't appear to be much simpler. Is it faster somehow?

If that's a viable alternative (i.e. it's correct and similarly fast), I would rather simply replace the implementation by yours and have it work on both Posix and Windows.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list