Skip to content

<filesystem>: Set the error code for fs::absolute("", ec).#6132

Merged
StephanTLavavej merged 3 commits into
microsoft:mainfrom
leejy12:6120-filesystem-absolute
Mar 31, 2026
Merged

<filesystem>: Set the error code for fs::absolute("", ec).#6132
StephanTLavavej merged 3 commits into
microsoft:mainfrom
leejy12:6120-filesystem-absolute

Conversation

@leejy12
Copy link
Copy Markdown
Contributor

@leejy12 leejy12 commented Mar 2, 2026

Closes #6120

fs::absolute("", ec) was returning an empty path, indicating an error has occurred, but didn't set ec to the appropriate error_code.

libstdc++ treats this call as an error, whereas libc++ treats it as a success case.

[fs.op.absolute#1] says the function "Composes an absolute path referencing the same file system location as p according to the operating system". On Windows, GetFullPathNameW(L"", ...) fails, so following libstdc++’s behavior is more consistent with the standard wording.

This change updates the implementation to set ec.value() to 123 (ERROR_INVALID_NAME).

@leejy12 leejy12 requested a review from a team as a code owner March 2, 2026 23:53
@github-project-automation github-project-automation Bot moved this to Initial Review in STL Code Reviews Mar 2, 2026
@YexuanXiao
Copy link
Copy Markdown
Contributor

YexuanXiao commented Mar 3, 2026

The error code for calling CreateFile with an empty string is ERROR_PATH_NOT_FOUND (3). Is it more appropriate to use it? This means the result obtained when not converting it to an absolute path.

@leejy12
Copy link
Copy Markdown
Contributor Author

leejy12 commented Mar 3, 2026 •
edited
Loading

@YexuanXiao I intended to let __std_fs_get_full_path_name populate it automatically.

STL/stl/src/filesystem.cpp

Lines 156 to 160 in 117ca96

[[nodiscard]] __std_ulong_and_error __stdcall __std_fs_get_full_path_name(_In_z_ const wchar_t* _Source,
_In_ unsigned long _Target_size, _Out_writes_z_(_Target_size) wchar_t* _Target) noexcept { // calls GetFullPathNameW
const auto _Result = GetFullPathNameW(_Source, _Target_size, _Target, nullptr);
return {_Result, _Result == 0 ? __std_win_error{GetLastError()} : __std_win_error::_Success};
}

But libstdc++ sets the error to EINVAL (22), so a better error may be ERROR_INVALID_PARAMETER (87). I think if we just return this when _Input._Text.empty() is true, we can avoid calling the Win32 API.

@StephanTLavavej StephanTLavavej added bug Something isn't working filesystem C++17 filesystem labels Mar 3, 2026
@StephanTLavavej StephanTLavavej self-assigned this Mar 30, 2026
@StephanTLavavej
Copy link
Copy Markdown
Member

StephanTLavavej commented Mar 31, 2026

I like the behavior of allowing empty strings through to the Windows API and returning whatever it returns, instead of attempting to synthesize our own error code.

@StephanTLavavej StephanTLavavej removed their assignment Mar 31, 2026
@StephanTLavavej StephanTLavavej moved this from Initial Review to Ready To Merge in STL Code Reviews Mar 31, 2026
@StephanTLavavej
Copy link
Copy Markdown
Member

StephanTLavavej commented Mar 31, 2026

I'm mirroring this to the MSVC-internal repo. Please notify me if any further changes are pushed, otherwise no action is required.

@StephanTLavavej StephanTLavavej moved this from Ready To Merge to Merging in STL Code Reviews Mar 31, 2026
@StephanTLavavej StephanTLavavej merged commit 2fe7911 into microsoft:main Mar 31, 2026
49 checks passed
@github-project-automation github-project-automation Bot moved this from Merging to Done in STL Code Reviews Mar 31, 2026
@StephanTLavavej
Copy link
Copy Markdown
Member

StephanTLavavej commented Mar 31, 2026

Thanks for investigating this libcxx failure and fixing our implementation! 💾 🛠️ 🎉

@leejy12 leejy12 deleted the 6120-filesystem-absolute branch March 31, 2026 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

No one assigned

Labels

bug Something isn't working filesystem C++17 filesystem

Projects

STL Code Reviews
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

<filesystem>: workaround dubious absolute("", ec) test case in libcxx suite

3 participants

@leejy12 @YexuanXiao @StephanTLavavej