Skip to content

windows: open files via std::filesystem::path so libc++ accepts wide paths#1084

Open
jcelerier wants to merge 1 commit into
microsoft:mainfrom
ossia:fix-windows-libcxx-file_sys
Open

windows: open files via std::filesystem::path so libc++ accepts wide paths#1084
jcelerier wants to merge 1 commit into
microsoft:mainfrom
ossia:fix-windows-libcxx-file_sys

Conversation

@jcelerier

Copy link
Copy Markdown

base/file_sys.h's path::open() constructs a std::ifstream directly from the std::wstring w_path_ on Windows:

#ifdef _WIN32
    return std::ifstream(w_path_, mode);

That basic_ifstream overload taking a wide string is a Microsoft-STL extension. libc++ (the clang Windows toolchain) does not provide it, so the header fails to compile:

error: no matching constructor for initialization of 'std::ifstream'
   54 |     return std::ifstream(w_path_, mode);
   note: no known conversion from 'const std::wstring' to 'const char *'

This fixes it by constructing a std::filesystem::path from the wide string and using basic_ifstream's filesystem::path constructor, which is standard C++17 and honoured by libc++, libstdc++ and MSVC STL alike — while still preserving the UTF-16 path for Unicode filenames.

Fixes #1071 (and the earlier #985).

Copilot AI review requested due to automatic review settings June 24, 2026 10:23
@jcelerier jcelerier requested a review from a team as a code owner June 24, 2026 10:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Windows builds using clang/libc++ by avoiding the non-standard MSVC-only std::ifstream(std::wstring, ...) overload, instead opening files via the standard C++17 std::ifstream(std::filesystem::path, ...) path-based constructor to preserve UTF-16 filenames.

Changes:

  • Add <filesystem> to support standard path-based stream constructors.
  • Update ort_extensions::path::open() on Windows to construct an std::ifstream from std::filesystem::path(w_path_).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jcelerier added a commit to ossia/score-addon-onnx that referenced this pull request Jun 24, 2026
The onnxruntime-extensions Windows build was force-disabled because the
upstream base/file_sys.h does not compile with clang + libc++ (the ossia
Windows toolchain): it constructs a std::ifstream from a std::wstring, an
overload only MSVC's STL provides (issue #985 / #1071).

Point the submodule at the ossia fork carrying the one-line fix -- open files
via std::filesystem::path, whose basic_ifstream ctor is standard C++17 and
honoured by libc++ -- filed upstream as microsoft/onnxruntime-extensions#1084.
With that, drop the WIN32 guard so FastVLM/QwenLLM build on every non-wasm
back-end, score and standalone alike. (Windows itself is CI-verified-pending;
Linux standalone still builds pyfastvlm.so + pyqwenllm.so.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ieWXB1cvaTVuVGuEmyKKP
…paths

base/file_sys.h's path::open() built a std::ifstream straight from the
std::wstring w_path_ on Windows. That ctor overload is a Microsoft-STL
extension; libc++ (the clang Windows toolchain) does not provide it, so the
header fails to compile with "no matching constructor for initialization of
'std::ifstream'". Construct a std::filesystem::path from the wide string
instead -- basic_ifstream's filesystem::path ctor is standard C++17, honoured
by libc++/libstdc++/MSVC STL, and still preserves the UTF-16 path.

Fixes microsoft#1071.
@jcelerier jcelerier force-pushed the fix-windows-libcxx-file_sys branch from b9652aa to 7d48543 Compare June 24, 2026 19:34
jcelerier added a commit to ossia/score-addon-onnx that referenced this pull request Jun 26, 2026
The onnxruntime-extensions Windows build was force-disabled because the
upstream base/file_sys.h does not compile with clang + libc++ (the ossia
Windows toolchain): it constructs a std::ifstream from a std::wstring, an
overload only MSVC's STL provides (issue #985 / #1071).

Point the submodule at the ossia fork carrying the one-line fix -- open files
via std::filesystem::path, whose basic_ifstream ctor is standard C++17 and
honoured by libc++ -- filed upstream as microsoft/onnxruntime-extensions#1084.
With that, drop the WIN32 guard so FastVLM/QwenLLM build on every non-wasm
back-end, score and standalone alike. (Windows itself is CI-verified-pending;
Linux standalone still builds pyfastvlm.so + pyqwenllm.so.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ieWXB1cvaTVuVGuEmyKKP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

windows: clang: file_sys.h does not work

2 participants