public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: [committed 1/3] libstdc++: Simplify filesystem::path SFINAE constraints
Date: Sat, 23 May 2020 09:42:06 +0100	[thread overview]
Message-ID: <20200523084206.GR2678@redhat.com> (raw)
In-Reply-To: <20200523084043.GQ2678@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

This replaces the filesystem::__detail::_Path SFINAE helper with two
separate helpers, _Path and _Path2. This avoids having one helper which
tries to check two different sets of requirements.

The _Path helper now uses variable templates instead of a set of
overloaded functions to detect specializations of basic_string or
basic_string_view.

The __not_<is_void<remove_pointer_t<_Tp1>> check is not necessary in
C++20 because iterator_traits<void*> is now empty. For C++17 replace
that check with a __safe_iterator_traits helper with partial
specializations for void pointers.

Finally, the __is_encoded_char check no longer uses remove_const_t,
which means that iterators with a const value_type will no longer be
accepted as arguments for path creation. Such iterators resulted in
undefined behaviour anyway, so it's still conforming to reject them in
the constraint checks.

         * include/bits/fs_path.h (filesystem::__detail::__is_encoded_char):
         Replace alias template with variable template. Don't remove const.
         (filesystem::__detail::__is_path_src): Replace overloaded function
         template with variable template and specializations.
         (filesystem::__detail::__is_path_iter_src): Replace alias template
         with class template.
         (filesystem::__detail::_Path): Use __is_path_src. Remove support for
         iterator pairs.
         (filesystem::__detail::_Path2): New alias template for checking
         InputIterator requirements.
         (filesystem::__detail::__constructible_from): Remove.
         (filesystem::path): Replace _Path<Iter, Iter> with _Path2<Iter>.
         * testsuite/27_io/filesystem/path/construct/80762.cc: Check with two
         constructor arguments of void and void* types.

Tested powerpc64le-linux, committed to master.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 10405 bytes --]

commit 988b853f9c829742907ae22ac66de56facfc7bc5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sat May 23 07:28:40 2020 +0100

    libstdc++: Simplify filesystem::path SFINAE constraints
    
    This replaces the filesystem::__detail::_Path SFINAE helper with two
    separate helpers, _Path and _Path2. This avoids having one helper which
    tries to check two different sets of requirements.
    
    The _Path helper now uses variable templates instead of a set of
    overloaded functions to detect specializations of basic_string or
    basic_string_view.
    
    The __not_<is_void<remove_pointer_t<_Tp1>> check is not necessary in
    C++20 because iterator_traits<void*> is now empty. For C++17 replace
    that check with a __safe_iterator_traits helper with partial
    specializations for void pointers.
    
    Finally, the __is_encoded_char check no longer uses remove_const_t,
    which means that iterators with a const value_type will no longer be
    accepted as arguments for path creation. Such iterators resulted in
    undefined behaviour anyway, so it's still conforming to reject them in
    the constraint checks.
    
            * include/bits/fs_path.h (filesystem::__detail::__is_encoded_char):
            Replace alias template with variable template. Don't remove const.
            (filesystem::__detail::__is_path_src): Replace overloaded function
            template with variable template and specializations.
            (filesystem::__detail::__is_path_iter_src): Replace alias template
            with class template.
            (filesystem::__detail::_Path): Use __is_path_src. Remove support for
            iterator pairs.
            (filesystem::__detail::_Path2): New alias template for checking
            InputIterator requirements.
            (filesystem::__detail::__constructible_from): Remove.
            (filesystem::path): Replace _Path<Iter, Iter> with _Path2<Iter>.
            * testsuite/27_io/filesystem/path/construct/80762.cc: Check with two
            constructor arguments of void and void* types.

diff --git a/libstdc++-v3/include/bits/fs_path.h b/libstdc++-v3/include/bits/fs_path.h
index ee6ab15cc4c..5a998284a99 100644
--- a/libstdc++-v3/include/bits/fs_path.h
+++ b/libstdc++-v3/include/bits/fs_path.h
@@ -73,58 +73,87 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 namespace __detail
 {
   template<typename _CharT>
-    using __is_encoded_char = __is_one_of<remove_const_t<_CharT>,
-	  char,
+    inline constexpr bool __is_encoded_char = false;
+  template<>
+    inline constexpr bool __is_encoded_char<char> = true;
 #ifdef _GLIBCXX_USE_CHAR8_T
-	  char8_t,
+  template<>
+    inline constexpr bool __is_encoded_char<char8_t> = true;
 #endif
 #if _GLIBCXX_USE_WCHAR_T
-	  wchar_t,
+  template<>
+    inline constexpr bool __is_encoded_char<wchar_t> = true;
 #endif
-	  char16_t, char32_t>;
-
-  template<typename _Iter,
-	   typename _Iter_traits = std::iterator_traits<_Iter>>
-    using __is_path_iter_src
-      = __and_<__is_encoded_char<typename _Iter_traits::value_type>,
-	       std::is_base_of<std::input_iterator_tag,
-			       typename _Iter_traits::iterator_category>>;
+  template<>
+    inline constexpr bool __is_encoded_char<char16_t> = true;
+  template<>
+    inline constexpr bool __is_encoded_char<char32_t> = true;
 
+#if __cpp_concepts >= 201907L
   template<typename _Iter>
-    static __is_path_iter_src<_Iter>
-    __is_path_src(_Iter, int);
-
-  template<typename _CharT, typename _Traits, typename _Alloc>
-    static __is_encoded_char<_CharT>
-    __is_path_src(const basic_string<_CharT, _Traits, _Alloc>&, int);
-
-  template<typename _CharT, typename _Traits>
-    static __is_encoded_char<_CharT>
-    __is_path_src(const basic_string_view<_CharT, _Traits>&, int);
-
-  template<typename _Unknown>
-    static std::false_type
-    __is_path_src(const _Unknown&, ...);
-
-  template<typename _Tp1, typename _Tp2>
-    struct __constructible_from;
-
+    using __safe_iterator_traits = std::iterator_traits<_Iter>;
+#else
   template<typename _Iter>
-    struct __constructible_from<_Iter, _Iter>
-    : __is_path_iter_src<_Iter>
+    struct __safe_iterator_traits : std::iterator_traits<_Iter>
+    { };
+
+  // Protect against ill-formed iterator_traits specializations in C++17
+  template<> struct __safe_iterator_traits<void*> { };
+  template<> struct __safe_iterator_traits<const void*> { };
+  template<> struct __safe_iterator_traits<volatile void*> { };
+  template<> struct __safe_iterator_traits<const volatile void*> { };
+#endif
+
+  template<typename _Iter_traits, typename = void>
+    struct __is_path_iter_src
+    : false_type
+    { };
+
+  template<typename _Iter_traits>
+    struct __is_path_iter_src<_Iter_traits,
+			      void_t<typename _Iter_traits::value_type>>
+    : bool_constant<__is_encoded_char<typename _Iter_traits::value_type>>
     { };
 
   template<typename _Source>
-    struct __constructible_from<_Source, void>
-    : decltype(__is_path_src(std::declval<_Source>(), 0))
-    { };
+    inline constexpr bool __is_path_src
+      = __is_path_iter_src<iterator_traits<decay_t<_Source>>>::value;
 
-  template<typename _Tp1, typename _Tp2 = void>
-    using _Path = typename
-      std::enable_if<__and_<__not_<is_same<remove_cv_t<_Tp1>, path>>,
-			    __not_<is_void<remove_pointer_t<_Tp1>>>,
-			    __constructible_from<_Tp1, _Tp2>>::value,
-		     path>::type;
+  template<>
+    inline constexpr bool __is_path_src<path> = false;
+
+  template<>
+    inline constexpr bool __is_path_src<volatile path> = false;
+
+  template<>
+    inline constexpr bool __is_path_src<void*> = false;
+
+  template<>
+    inline constexpr bool __is_path_src<const void*> = false;
+
+  template<>
+    inline constexpr bool __is_path_src<volatile void*> = false;
+
+  template<>
+    inline constexpr bool __is_path_src<const volatile void*> = false;
+
+  template<typename _CharT, typename _Traits, typename _Alloc>
+    inline constexpr bool
+      __is_path_src<basic_string<_CharT, _Traits, _Alloc>>
+	= __is_encoded_char<_CharT>;
+
+  template<typename _CharT, typename _Traits>
+    inline constexpr bool
+      __is_path_src<basic_string_view<_CharT, _Traits>>
+	= __is_encoded_char<_CharT>;
+
+  // SFINAE constraint for Source parameters as required by [fs.path.req].
+  template<typename _Tp>
+    using _Path = enable_if_t<__is_path_src<_Tp>, path>;
+
+  // SFINAE constraint for InputIterator parameters as required by [fs.req].
+  template<typename _Iter, typename _Tr = __safe_iterator_traits<_Iter>>
+    using _Path2 = enable_if_t<__is_path_iter_src<_Tr>::value, path>;
 
   template<typename _Source>
     static _Source
@@ -227,7 +256,7 @@ namespace __detail
       { _M_split_cmpts(); }
 
     template<typename _InputIterator,
-	     typename _Require = __detail::_Path<_InputIterator, _InputIterator>>
+	     typename _Require = __detail::_Path2<_InputIterator>>
       path(_InputIterator __first, _InputIterator __last, format = auto_format)
       : _M_pathname(_S_convert(__first, __last))
       { _M_split_cmpts(); }
@@ -241,8 +270,8 @@ namespace __detail
       { _M_split_cmpts(); }
 
     template<typename _InputIterator,
-	     typename _Require = __detail::_Path<_InputIterator, _InputIterator>,
-	     typename _Require2 = __detail::__value_type_is_char<_InputIterator>>
+	     typename _Require = __detail::_Path2<_InputIterator>,
+	     typename _Req2 = __detail::__value_type_is_char<_InputIterator>>
       path(_InputIterator __first, _InputIterator __last, const locale& __loc,
 	   format = auto_format)
       : _M_pathname(_S_convert_loc(__first, __last, __loc))
@@ -268,7 +297,7 @@ namespace __detail
       { return *this = path(__source); }
 
     template<typename _InputIterator>
-      __detail::_Path<_InputIterator, _InputIterator>&
+      __detail::_Path2<_InputIterator>&
       assign(_InputIterator __first, _InputIterator __last)
       { return *this = path(__first, __last); }
 
@@ -295,7 +324,7 @@ namespace __detail
       }
 
     template<typename _InputIterator>
-      __detail::_Path<_InputIterator, _InputIterator>&
+      __detail::_Path2<_InputIterator>&
       append(_InputIterator __first, _InputIterator __last)
       {
 	_M_append(_S_convert(__first, __last));
@@ -315,7 +344,7 @@ namespace __detail
       operator+=(_Source const& __x) { return concat(__x); }
 
     template<typename _CharT>
-      __detail::_Path<_CharT*, _CharT*>&
+      __detail::_Path2<_CharT*>&
       operator+=(_CharT __x);
 
     template<typename _Source>
@@ -328,7 +357,7 @@ namespace __detail
       }
 
     template<typename _InputIterator>
-      __detail::_Path<_InputIterator, _InputIterator>&
+      __detail::_Path2<_InputIterator>&
       concat(_InputIterator __first, _InputIterator __last)
       {
 	_M_concat(_S_convert(__first, __last));
@@ -695,7 +724,7 @@ namespace __detail
    * @relates std::filesystem::path
    */
   template<typename _InputIterator,
-	   typename _Require = __detail::_Path<_InputIterator, _InputIterator>,
+	   typename _Require = __detail::_Path2<_InputIterator>,
 	   typename _CharT
 	     = __detail::__value_type_is_char_or_char8_t<_InputIterator>>
     inline path
@@ -1000,7 +1029,7 @@ namespace __detail
   }
 
   template<typename _CharT>
-    inline __detail::_Path<_CharT*, _CharT*>&
+    inline __detail::_Path2<_CharT*>&
     path::operator+=(_CharT __x)
     {
       auto* __addr = std::__addressof(__x);
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/construct/80762.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/80762.cc
index bfc1e125e0c..2f37b663f26 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/path/construct/80762.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/80762.cc
@@ -37,3 +37,9 @@ static_assert( !std::is_constructible_v<path, const volatile void*> );
 static_assert( !std::is_constructible_v<path, void*&> );
 static_assert( !std::is_constructible_v<path, void* const&> );
 static_assert( !std::is_constructible_v<path, const void* const&> );
+
+static_assert( !std::is_constructible_v<path, void, void> );
+static_assert( !std::is_constructible_v<path, void*, void*> );
+static_assert( !std::is_constructible_v<path, const void*, void*> );
+static_assert( !std::is_constructible_v<path, volatile void*, void*> );
+static_assert( !std::is_constructible_v<path, const volatile void*, void*> );

  reply	other threads:[~2020-05-23  8:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-23  8:40 [committed 0/3] libstdc++: Refactor filesystem::path string conversions Jonathan Wakely
2020-05-23  8:42 ` Jonathan Wakely [this message]
2020-05-23  8:43 ` [committed 2/3] libstdc++: Remove incorrect static specifiers Jonathan Wakely
2020-05-23  8:44 ` [committed 3/3] libstdc++: Refactor filesystem::path string conversions Jonathan Wakely
2020-06-01 23:14   ` Jonathan Wakely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200523084206.GR2678@redhat.com \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).