* [committed] libstdc++: Fix filesystem::path comparisons for C++23
@ 2021-06-11 18:24 Jonathan Wakely
2021-06-14 12:58 ` Christophe Lyon
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2021-06-11 18:24 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]
In C++23 there is a basic_string_view(Range&&) constructor, which is
constrained to take a range (specifically, a contiguous_range). When the
filesystem::path comparison operators call lhs.compare(rhs) the overload
taking a string_view is considered, which means checking whether path
satisfies the range concept. That satisfaction result changes depending
whether path::iterator is complete, which is ill-formed; no diagnostic
required. To avoid the problem, this change ensures that the overload
resolution is performed in a context where path::iterator is complete
and the range concept is satisfied. (The result of overload resolution
is always that the compare(const path&) overload is the best match, but
we still have to consider the compare(basic_string_view<value_type>) one
to decide if it even participates in overload resolution).
For std::filesystem::path we can't define the comparison operators later
in the file, because they are hidden friends, so a new helper is
introduced that gets defined when everything else is complete.
For std::experimental::filesystem::path we can just move the definitions
of the comparison operators later in the file.
Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
libstdc++-v3/ChangeLog:
* include/bits/fs_path.h (operator==, operator<=>): Use new
_S_compare function.
(path::_S_compare): New function to call path::compare in a
context where path::iterator is complete.
* include/experimental/bits/fs_path.h (operator<, operator==):
Define after path::iterator is complete.
* testsuite/27_io/filesystem/path/native/conv_c++23.cc: New
test.
* testsuite/experimental/filesystem/path/native/conv_c++23.cc:
New test.
Tested powerpc64le-linux. Committed to trunk.
This also needs to be backported to gcc-11 where the string_view
constructor is present for C++23 mode.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 6656 bytes --]
commit 1e690757d30775ed340a368b9a9463b2ad68de01
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Fri Jun 11 19:18:11 2021
libstdc++: Fix filesystem::path comparisons for C++23
In C++23 there is a basic_string_view(Range&&) constructor, which is
constrained to take a range (specifically, a contiguous_range). When the
filesystem::path comparison operators call lhs.compare(rhs) the overload
taking a string_view is considered, which means checking whether path
satisfies the range concept. That satisfaction result changes depending
whether path::iterator is complete, which is ill-formed; no diagnostic
required. To avoid the problem, this change ensures that the overload
resolution is performed in a context where path::iterator is complete
and the range concept is satisfied. (The result of overload resolution
is always that the compare(const path&) overload is the best match, but
we still have to consider the compare(basic_string_view<value_type>) one
to decide if it even participates in overload resolution).
For std::filesystem::path we can't define the comparison operators later
in the file, because they are hidden friends, so a new helper is
introduced that gets defined when everything else is complete.
For std::experimental::filesystem::path we can just move the definitions
of the comparison operators later in the file.
Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
libstdc++-v3/ChangeLog:
* include/bits/fs_path.h (operator==, operator<=>): Use new
_S_compare function.
(path::_S_compare): New function to call path::compare in a
context where path::iterator is complete.
* include/experimental/bits/fs_path.h (operator<, operator==):
Define after path::iterator is complete.
* testsuite/27_io/filesystem/path/native/conv_c++23.cc: New
test.
* testsuite/experimental/filesystem/path/native/conv_c++23.cc:
New test.
diff --git a/libstdc++-v3/include/bits/fs_path.h b/libstdc++-v3/include/bits/fs_path.h
index ad2518f414c..5e285204527 100644
--- a/libstdc++-v3/include/bits/fs_path.h
+++ b/libstdc++-v3/include/bits/fs_path.h
@@ -513,13 +513,13 @@ namespace __detail
/// Compare paths
friend bool operator==(const path& __lhs, const path& __rhs) noexcept
- { return __lhs.compare(__rhs) == 0; }
+ { return path::_S_compare(__lhs, __rhs) == 0; }
#if __cpp_lib_three_way_comparison
/// Compare paths
friend strong_ordering
operator<=>(const path& __lhs, const path& __rhs) noexcept
- { return __lhs.compare(__rhs) <=> 0; }
+ { return path::_S_compare(__lhs, __rhs) <=> 0; }
#else
/// Compare paths
friend bool operator!=(const path& __lhs, const path& __rhs) noexcept
@@ -627,6 +627,11 @@ namespace __detail
static basic_string<_CharT, _Traits, _Allocator>
_S_str_convert(basic_string_view<value_type>, const _Allocator&);
+ // Returns lhs.compare(rhs), but defined after path::iterator is complete.
+ __attribute__((__always_inline__))
+ static int
+ _S_compare(const path& __lhs, const path& __rhs) noexcept;
+
void _M_split_cmpts();
_Type _M_type() const noexcept { return _M_cmpts.type(); }
@@ -1330,6 +1335,14 @@ namespace __detail
return _M_at_end == __rhs._M_at_end;
}
+ // Define this now that path and path::iterator are complete.
+ // It needs to consider the string_view(Range&&) constructor during
+ // overload resolution, which depends on whether range<path> is satisfied,
+ // which depends on whether path::iterator is complete.
+ inline int
+ path::_S_compare(const path& __lhs, const path& __rhs) noexcept
+ { return __lhs.compare(__rhs); }
+
/// @} group filesystem
_GLIBCXX_END_NAMESPACE_CXX11
} // namespace filesystem
diff --git a/libstdc++-v3/include/experimental/bits/fs_path.h b/libstdc++-v3/include/experimental/bits/fs_path.h
index 1ecf2f3a7bd..1fb84ec1e78 100644
--- a/libstdc++-v3/include/experimental/bits/fs_path.h
+++ b/libstdc++-v3/include/experimental/bits/fs_path.h
@@ -551,8 +551,7 @@ namespace __detail
size_t hash_value(const path& __p) noexcept;
/// Compare paths
- inline bool operator<(const path& __lhs, const path& __rhs) noexcept
- { return __lhs.compare(__rhs) < 0; }
+ inline bool operator<(const path& __lhs, const path& __rhs) noexcept;
/// Compare paths
inline bool operator<=(const path& __lhs, const path& __rhs) noexcept
@@ -567,8 +566,7 @@ namespace __detail
{ return !(__lhs < __rhs); }
/// Compare paths
- inline bool operator==(const path& __lhs, const path& __rhs) noexcept
- { return __lhs.compare(__rhs) == 0; }
+ inline bool operator==(const path& __lhs, const path& __rhs) noexcept;
/// Compare paths
inline bool operator!=(const path& __lhs, const path& __rhs) noexcept
@@ -1275,6 +1273,16 @@ namespace __detail
return _M_at_end == __rhs._M_at_end;
}
+ // Define these now that path and path::iterator are complete.
+ // They needs to consider the string_view(Range&&) constructor during
+ // overload resolution, which depends on whether range<path> is satisfied,
+ // which depends on whether path::iterator is complete.
+ inline bool operator<(const path& __lhs, const path& __rhs) noexcept
+ { return __lhs.compare(__rhs) < 0; }
+
+ inline bool operator==(const path& __lhs, const path& __rhs) noexcept
+ { return __lhs.compare(__rhs) == 0; }
+
/// @} group filesystem-ts
_GLIBCXX_END_NAMESPACE_CXX11
} // namespace v1
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/native/conv_c++23.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/native/conv_c++23.cc
new file mode 100644
index 00000000000..b4efed2bd47
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/native/conv_c++23.cc
@@ -0,0 +1,12 @@
+// { dg-options "-std=gnu++23" }
+// { dg-do compile { target c++23 } }
+
+#include <filesystem>
+
+void
+test01()
+{
+ using std::filesystem::path;
+ path p;
+ path::string_type s(p);
+}
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/path/native/conv_c++23.cc b/libstdc++-v3/testsuite/experimental/filesystem/path/native/conv_c++23.cc
new file mode 100644
index 00000000000..f3c3f884965
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/path/native/conv_c++23.cc
@@ -0,0 +1,12 @@
+// { dg-options "-std=gnu++23" }
+// { dg-do compile { target c++23 } }
+
+#include <experimental/filesystem>
+
+void
+test01()
+{
+ using std::experimental::filesystem::path;
+ path p;
+ path::string_type s(p);
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [committed] libstdc++: Fix filesystem::path comparisons for C++23
2021-06-11 18:24 [committed] libstdc++: Fix filesystem::path comparisons for C++23 Jonathan Wakely
@ 2021-06-14 12:58 ` Christophe Lyon
2021-06-14 14:19 ` Jonathan Wakely
0 siblings, 1 reply; 3+ messages in thread
From: Christophe Lyon @ 2021-06-14 12:58 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc Patches
Hi Jonathan,
On Fri, 11 Jun 2021 at 20:25, Jonathan Wakely via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> In C++23 there is a basic_string_view(Range&&) constructor, which is
> constrained to take a range (specifically, a contiguous_range). When the
> filesystem::path comparison operators call lhs.compare(rhs) the overload
> taking a string_view is considered, which means checking whether path
> satisfies the range concept. That satisfaction result changes depending
> whether path::iterator is complete, which is ill-formed; no diagnostic
> required. To avoid the problem, this change ensures that the overload
> resolution is performed in a context where path::iterator is complete
> and the range concept is satisfied. (The result of overload resolution
> is always that the compare(const path&) overload is the best match, but
> we still have to consider the compare(basic_string_view<value_type>) one
> to decide if it even participates in overload resolution).
>
> For std::filesystem::path we can't define the comparison operators later
> in the file, because they are hidden friends, so a new helper is
> introduced that gets defined when everything else is complete.
>
> For std::experimental::filesystem::path we can just move the definitions
> of the comparison operators later in the file.
>
> Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/fs_path.h (operator==, operator<=>): Use new
> _S_compare function.
> (path::_S_compare): New function to call path::compare in a
> context where path::iterator is complete.
> * include/experimental/bits/fs_path.h (operator<, operator==):
> Define after path::iterator is complete.
> * testsuite/27_io/filesystem/path/native/conv_c++23.cc: New
> test.
> * testsuite/experimental/filesystem/path/native/conv_c++23.cc:
> New test.
>
> Tested powerpc64le-linux. Committed to trunk.
>
> This also needs to be backported to gcc-11 where the string_view
> constructor is present for C++23 mode.
>
The new test experimental/filesystem/path/native/conv_c++23.cc
fails on arm-eabi and aarch64-elf:
/libstdc++-v3/testsuite/experimental/filesystem/path/native/conv_c++23.cc:4:
fatal error: experimental/filesystem: No such file or directory
(both trunk and gcc-11)
I guess that's easy to fix? :-)
Thanks,
Christophe
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [committed] libstdc++: Fix filesystem::path comparisons for C++23
2021-06-14 12:58 ` Christophe Lyon
@ 2021-06-14 14:19 ` Jonathan Wakely
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2021-06-14 14:19 UTC (permalink / raw)
To: Christophe Lyon; +Cc: libstdc++, gcc Patches
[-- Attachment #1: Type: text/plain, Size: 2658 bytes --]
On Mon, 14 Jun 2021 at 13:58, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
>
> Hi Jonathan,
>
> On Fri, 11 Jun 2021 at 20:25, Jonathan Wakely via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > In C++23 there is a basic_string_view(Range&&) constructor, which is
> > constrained to take a range (specifically, a contiguous_range). When the
> > filesystem::path comparison operators call lhs.compare(rhs) the overload
> > taking a string_view is considered, which means checking whether path
> > satisfies the range concept. That satisfaction result changes depending
> > whether path::iterator is complete, which is ill-formed; no diagnostic
> > required. To avoid the problem, this change ensures that the overload
> > resolution is performed in a context where path::iterator is complete
> > and the range concept is satisfied. (The result of overload resolution
> > is always that the compare(const path&) overload is the best match, but
> > we still have to consider the compare(basic_string_view<value_type>) one
> > to decide if it even participates in overload resolution).
> >
> > For std::filesystem::path we can't define the comparison operators later
> > in the file, because they are hidden friends, so a new helper is
> > introduced that gets defined when everything else is complete.
> >
> > For std::experimental::filesystem::path we can just move the definitions
> > of the comparison operators later in the file.
> >
> > Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/bits/fs_path.h (operator==, operator<=>): Use new
> > _S_compare function.
> > (path::_S_compare): New function to call path::compare in a
> > context where path::iterator is complete.
> > * include/experimental/bits/fs_path.h (operator<, operator==):
> > Define after path::iterator is complete.
> > * testsuite/27_io/filesystem/path/native/conv_c++23.cc: New
> > test.
> > * testsuite/experimental/filesystem/path/native/conv_c++23.cc:
> > New test.
> >
> > Tested powerpc64le-linux. Committed to trunk.
> >
> > This also needs to be backported to gcc-11 where the string_view
> > constructor is present for C++23 mode.
> >
>
> The new test experimental/filesystem/path/native/conv_c++23.cc
> fails on arm-eabi and aarch64-elf:
> /libstdc++-v3/testsuite/experimental/filesystem/path/native/conv_c++23.cc:4:
> fatal error: experimental/filesystem: No such file or directory
> (both trunk and gcc-11)
>
> I guess that's easy to fix? :-)
Yes, fixed on trunk now by the attached patch. Tests running for the
gcc-11 backport.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 871 bytes --]
commit a40d2293a7a417fc7711f8f4e52dbd4c48c2a43f
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon Jun 14 14:22:07 2021
libstdc++: Only run Filesystem TS test if supported
libstdc++-v3/ChangeLog:
* testsuite/experimental/filesystem/path/native/conv_c++23.cc:
Add dg-require-filesystem-ts directive.
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/path/native/conv_c++23.cc b/libstdc++-v3/testsuite/experimental/filesystem/path/native/conv_c++23.cc
index f3c3f884965..81f57549ac9 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/path/native/conv_c++23.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/path/native/conv_c++23.cc
@@ -1,5 +1,6 @@
// { dg-options "-std=gnu++23" }
// { dg-do compile { target c++23 } }
+// { dg-require-filesystem-ts "" }
#include <experimental/filesystem>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-14 14:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11 18:24 [committed] libstdc++: Fix filesystem::path comparisons for C++23 Jonathan Wakely
2021-06-14 12:58 ` Christophe Lyon
2021-06-14 14:19 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).