[libcxx-commits] [libcxx] [libc++][filesystem] Report an error for canonical("") (PR #215031)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Aug 8 15:52:55 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Yordan Vásquez (vyordan)
<details>
<summary>Changes</summary>
canonical(p) requires !exists(p) to be an error per [fs.op.canonical]. An empty path never refers to an existing file (exists(path()) is always false, matching how a POSIX stat("") call fails with ENOENT), so canonical("") must report an error instead of silently returning the current directory.
The current implementation absolutizes p before checking whether it resolves to something real: __do_absolute(p) computes cwd / p, and appending an empty path via operator/= yields cwd itself (with a trailing separator), which does exist. This hides the required error, since no information survives about whether the original argument was empty. This is a corner case unique to the empty path: no other relative path can "disappear" when composed with the cwd.
Fix this at the source by checking orig_p.empty() in __canonical before absolutizing, using the original, unmodified argument.
absolute() is unaffected by design: [fs.op.absolute] explicitly discourages treating !exists(p) as an error, since it is a purely lexical composition (current_path() / p). This matches the existing absolute.pass.cpp expectation that absolute("") == current_path() / "".
weakly_canonical() is also unaffected in its observable behavior: [fs.op.weakly_canonical] never requires existence either. It used __canonical("", ec) internally as a shortcut relying on the old (buggy) behavior to resolve to the current directory; both internal call sites are updated to call __current_path() directly instead, producing the exact same result without depending on the now-corrected canonical(""). Since relative() and proximate() are built solely on top of weakly_canonical(), they require no changes either.
---
Full diff: https://github.com/llvm/llvm-project/pull/215031.diff
3 Files Affected:
- (modified) libcxx/docs/ReleaseNotes/22.rst (+7)
- (modified) libcxx/src/filesystem/operations.cpp (+20-2)
- (modified) libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp (+31)
``````````diff
diff --git a/libcxx/docs/ReleaseNotes/22.rst b/libcxx/docs/ReleaseNotes/22.rst
index 9f9927095a61f..97852fed4b4fe 100644
--- a/libcxx/docs/ReleaseNotes/22.rst
+++ b/libcxx/docs/ReleaseNotes/22.rst
@@ -128,6 +128,13 @@ Potentially breaking changes
``_LIBCPP_ABI_NO_ITERATOR_BASES``. If you are using this flag and care about ABI stability, you should set
``_LIBCPP_ABI_NO_REVERSE_ITERATOR_SECOND_MEMBER`` as well.
+- ``std::filesystem::canonical`` now reports an error (throwing ``filesystem_error``, or setting the
+ ``error_code`` for the overload that takes one) when called with an empty path, as required by
+ ``[fs.op.canonical]`` ("``!exists(p)`` is an error"), instead of silently returning the current
+ directory. ``std::filesystem::absolute``, ``std::filesystem::weakly_canonical``,
+ ``std::filesystem::relative`` and ``std::filesystem::proximate`` are unaffected and keep resolving an
+ empty path to the current directory, as required by the Standard. (`Github <https://llvm.org/PR77170>`__)
+
Announcements About Future Releases
-----------------------------------
diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index 3ff62c102d0ad..0cee6a1f4a038 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -99,6 +99,15 @@ path __canonical(path const& orig_p, error_code* ec) {
path cwd;
ErrorHandler<path> err("canonical", ec, &orig_p, &cwd);
+ // canonical() requires the path to exist ([fs.op.canonical]: "!exists(p) is
+ // an error"). An empty path never refers to an existing file -- exists("")
+ // is always false, since status("") would fail the same way a POSIX
+ // stat("") call fails with ENOENT. Absolutizing an empty path below would
+ // otherwise silently produce the (existing) current directory and hide
+ // this required error.
+ if (orig_p.empty())
+ return err.report(errc::no_such_file_or_directory);
+
path p = __do_absolute(orig_p, &cwd, ec);
#if (defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112) || defined(_WIN32)
std::unique_ptr<path::value_type, decltype(&::free)> hold(detail::realpath(p.c_str(), nullptr), &::free);
@@ -1042,8 +1051,12 @@ path __temp_directory_path(error_code* ec) {
path __weakly_canonical(const path& p, error_code* ec) {
ErrorHandler<path> err("weakly_canonical", ec, &p);
+ // weakly_canonical() never requires its argument to exist ([fs.op.weakly.canonical]),
+ // so an empty path -- which has no leading elements at all -- resolves to
+ // the current directory directly. This must not go through __canonical(""),
+ // which is now required to report an error for an empty path.
if (p.empty())
- return __canonical("", ec);
+ return __current_path(ec);
path result;
path tmp;
@@ -1069,7 +1082,12 @@ path __weakly_canonical(const path& p, error_code* ec) {
--PP;
}
if (PP.State_ == PathParser::PS_BeforeBegin) {
- result = __canonical("", &m_ec);
+ // None of the elements of p exist (or p was relative with no root and
+ // none of its ancestors exist either); the "leading elements of p that
+ // exist" are therefore empty, and canonical() of that degenerate,
+ // zero-element path is the current directory. As above, this must not
+ // be routed through __canonical(""), which now reports an error.
+ result = __current_path(&m_ec);
if (m_ec) {
return err.report(m_ec);
}
diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp
index 5323f3bc8337f..6a844b1bdf5a0 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.canonical/canonical.pass.cpp
@@ -92,6 +92,36 @@ static void test_dne_path()
}
}
+// https://llvm.org/PR77170
+// Per [fs.op.canonical], "!exists(p) is an error", and an empty path never
+// refers to an existing file (exists(path()) is always false). canonical("")
+// must therefore report an error instead of silently returning the current
+// directory.
+static void test_empty_path()
+{
+ std::error_code ec = GetTestEC();
+ {
+ const path ret = canonical(path{}, ec);
+ assert(ec != GetTestEC());
+ assert(ec);
+ assert(ret == path{});
+ }
+ {
+ TEST_THROWS_TYPE(filesystem_error, canonical(path{}));
+ }
+
+ ec = GetTestEC();
+ {
+ const path ret = canonical("", ec);
+ assert(ec != GetTestEC());
+ assert(ec);
+ assert(ret == path{});
+ }
+ {
+ TEST_THROWS_TYPE(filesystem_error, canonical(""));
+ }
+}
+
static void test_exception_contains_paths()
{
#ifndef TEST_HAS_NO_EXCEPTIONS
@@ -121,6 +151,7 @@ int main(int, char**) {
signature_test();
test_canonical();
test_dne_path();
+ test_empty_path();
test_exception_contains_paths();
return 0;
``````````
</details>
https://github.com/llvm/llvm-project/pull/215031
More information about the libcxx-commits
mailing list