[PATCH] D74477: [llvm-ar] Simplify Windows comparePaths NFCI

Andrew Ng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 13 10:43:10 PST 2020


andrewng added a comment.

In D74477#1874772 <https://reviews.llvm.org/D74477#1874772>, @rupprecht wrote:

> In D74477#1873998 <https://reviews.llvm.org/D74477#1873998>, @andrewng wrote:
>
> > In my modified version of widenPath...
>
>
> I second grimar's request for an associated test that covers this behavior, but are you saying that the issue is your modified version of `sys::path::widenPath` breaks some llvm-ar tests, and switching to `sys::windows::UTF8ToUTF16` will keep them passing? (If so, submitting w/o an additional test sounds fine to me)


My modified version of `sys::path::widenPath` does not affect any llvm-ar tests and actually should be less likely to cause any problems.

The key point is that the use of `sys::path::widenPath` is not required here and could possibly cause problems, where as using `sys::windows::UTF8ToUTF16` is simpler and more appropriate.


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

https://reviews.llvm.org/D74477





More information about the llvm-commits mailing list