[llvm] [Support] Use std::filesystem::remove_all() in remove_directories() (PR #119146)

Dmitry Vasilyev via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 9 04:24:25 PST 2024


================
@@ -1373,29 +1374,10 @@ std::error_code closeFile(file_t &F) {
 }
 
 std::error_code remove_directories(const Twine &path, bool IgnoreErrors) {
-  // Convert to utf-16.
-  SmallVector<wchar_t, 128> Path16;
-  std::error_code EC = widenPath(path, Path16);
-  if (EC && !IgnoreErrors)
-    return EC;
-
-  // SHFileOperation() accepts a list of paths, and so must be double null-
-  // terminated to indicate the end of the list.  The buffer is already null
-  // terminated, but since that null character is not considered part of the
-  // vector's size, pushing another one will just consume that byte.  So we
-  // need to push 2 null terminators.
-  Path16.push_back(0);
-  Path16.push_back(0);
-
-  SHFILEOPSTRUCTW shfos = {};
-  shfos.wFunc = FO_DELETE;
-  shfos.pFrom = Path16.data();
-  shfos.fFlags = FOF_NO_UI;
-
-  int result = ::SHFileOperationW(&shfos);
-  if (result != 0 && !IgnoreErrors)
-    return mapWindowsError(result);
-  return std::error_code();
+  const std::filesystem::path Path(path.str());
----------------
slydiman wrote:

MS STL calls
https://github.com/microsoft/STL/blob/89ca073a866d24db42b3f4c5e406be4b28a148f4/stl/inc/filesystem#L655
```
template <class _Src, enable_if_t<_Is_Source<_Src>, int> = 0>
path(const _Src& _Source, format = auto_format) : _Text(_Convert_Source_to_wide(_Source)) {
    // format has no meaning for this implementation, as the generic grammar is acceptable as a native path
}
```
https://github.com/microsoft/STL/blob/89ca073a866d24db42b3f4c5e406be4b28a148f4/stl/inc/filesystem#L280
```
template <class _Src, class _Conversion = _Normal_conversion>
_NODISCARD wstring _Convert_Source_to_wide(const _Src& _Source, _Conversion _Tag = {}) {
    return _Convert_stringoid_to_wide(_Stringoid_from_Source(_Source), _Tag);
}
```
https://github.com/microsoft/STL/blob/89ca073a866d24db42b3f4c5e406be4b28a148f4/stl/inc/filesystem#L193-L201
```
template <class _Conversion>
_NODISCARD wstring _Convert_stringoid_to_wide(const string_view _Input, _Conversion) {
    _STL_INTERNAL_STATIC_ASSERT(_Is_any_of_v<_Conversion, _Normal_conversion, _Utf8_conversion>);
     if constexpr (is_same_v<_Conversion, _Normal_conversion>) {
        return _Convert_narrow_to_wide(__std_fs_code_page(), _Input);
    } else {
        return _Convert_narrow_to_wide(__std_code_page::_Utf8, _Input);
    }
}
```
and `__std_fs_code_page()` returns CP_ACP if system's locale is English.
So, yes, it does not expect any non-ASCII symbols in the path.
I have updated the patch to use widenPath() and tested it with non-ASCII symbols locally.

https://github.com/llvm/llvm-project/pull/119146


More information about the llvm-commits mailing list