On Fri, Apr 4, 2025 at 11:08 AM Jonathan Wakely <jwak...@redhat.com> wrote:

> We can use the __glibcxx_string_view macro to guard the uses of
> std::string_view in <string>, instead of just checking the value of
> __cplusplus. It makes no practical difference because
> __glibcxx_string_view is defined for C++17 and up, but it makes it clear
> to readers that the lines guarded by that macro are features that depend
> on string_view.
>
> We could be more precise and check __glibcxx_string_view >= 201606L
> which is the value for the P0254R2 paper that integrated
> std::string_view with std::string, but I think just checking for the
> macro being defined is clear enough.
>
> We can also check __glibcxx_variant for the _Never_valueless_alt partial
> specialization.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/basic_string.h: Check __glibcxx_string_view and
>         __glibcxx_variant instead of __cplusplus >= 2017L.
>         * include/bits/cow_string.h: Likewise.
> ---
>
> Tested x86_64-linux.
>
LGTM. Thanks for changing that. As I mentioned before, I believe using
feature test macros
for guards, helps to navigate the code, especially in case of newcomers.

>
>  libstdc++-v3/include/bits/basic_string.h | 39 ++++++++++++------------
>  libstdc++-v3/include/bits/cow_string.h   | 30 +++++++++---------
>  2 files changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/basic_string.h
> b/libstdc++-v3/include/bits/basic_string.h
> index 86841cb2c5e..886e7e6b19e 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -45,7 +45,9 @@
>  #include <initializer_list>
>  #endif
>
> -#if __cplusplus >= 201703L
> +#include <bits/version.h>
> +
> +#ifdef __glibcxx_string_view // >= C++17
>  # include <string_view>
>  #endif
>
> @@ -53,7 +55,6 @@
>  # include <charconv>
>  #endif
>
> -#include <bits/version.h>
>
>  #if ! _GLIBCXX_USE_CXX11_ABI
>  # include "cow_string.h"
> @@ -146,7 +147,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>         return __p;
>        }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        // A helper type for avoiding boiler-plate.
>        typedef basic_string_view<_CharT, _Traits> __sv_type;
>
> @@ -788,7 +789,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>  #endif
>         }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Construct string from a substring of a string_view.
>         *  @param  __t   Source object convertible to string view.
> @@ -944,7 +945,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        }
>  #endif // C++11
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Set value to string constructed from a string_view.
>         *  @param  __svt  An object convertible to string_view.
> @@ -1439,7 +1440,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        { return this->append(__l.begin(), __l.size()); }
>  #endif // C++11
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Append a string_view.
>         *  @param __svt  An object convertible to string_view to be
> appended.
> @@ -1556,7 +1557,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>          append(_InputIterator __first, _InputIterator __last)
>          { return this->replace(end(), end(), __first, __last); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view
>        /**
>         *  @brief  Append a string_view.
>         *  @param __svt  An object convertible to string_view to be
> appended.
> @@ -1809,7 +1810,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        }
>  #endif // C++11
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Set value from a string_view.
>         *  @param __svt  The source object convertible to string_view.
> @@ -2090,7 +2091,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>         return iterator(_M_data() + __pos);
>        }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Insert a string_view.
>         *  @param __pos  Position in string to insert at.
> @@ -2542,7 +2543,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        { return this->replace(__i1, __i2, __l.begin(), __l.size()); }
>  #endif // C++11
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Replace range of characters with string_view.
>         *  @param __pos  The position to replace at.
> @@ -2741,7 +2742,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        _GLIBCXX_NOEXCEPT
>        { return this->find(__str.data(), __pos, __str.size()); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find position of a string_view.
>         *  @param __svt  The object convertible to string_view to locate.
> @@ -2807,7 +2808,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        _GLIBCXX_NOEXCEPT
>        { return this->rfind(__str.data(), __pos, __str.size()); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find last position of a string_view.
>         *  @param __svt  The object convertible to string_view to locate.
> @@ -2891,7 +2892,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        _GLIBCXX_NOEXCEPT
>        { return this->find_first_of(__str.data(), __pos, __str.size()); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find position of a character of a string_view.
>         *  @param __svt  An object convertible to string_view containing
> @@ -2980,7 +2981,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        _GLIBCXX_NOEXCEPT
>        { return this->find_last_of(__str.data(), __pos, __str.size()); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find last position of a character of string.
>         *  @param __svt  An object convertible to string_view containing
> @@ -3068,7 +3069,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        _GLIBCXX_NOEXCEPT
>        { return this->find_first_not_of(__str.data(), __pos,
> __str.size()); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find position of a character not in a string_view.
>         *  @param __svt  A object convertible to string_view containing
> @@ -3155,7 +3156,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>        _GLIBCXX_NOEXCEPT
>        { return this->find_last_not_of(__str.data(), __pos, __str.size());
> }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find last position of a character not in a string_view.
>         *  @param __svt  An object convertible to string_view containing
> @@ -3271,7 +3272,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>         return __r;
>        }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Compare to a string_view.
>         *  @param __svt An object convertible to string_view to compare
> against.
> @@ -4605,7 +4606,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>    constexpr
>  #endif
>    inline wstring
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>    __to_wstring_numeric(string_view __s)
>  #else
>    __to_wstring_numeric(const string& __s)
> @@ -4808,7 +4809,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    } // inline namespace literals
>  #endif // __glibcxx_string_udls
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_variant // >= C++17
>    namespace __detail::__variant
>    {
>      template<typename> struct _Never_valueless_alt; // see <variant>
> diff --git a/libstdc++-v3/include/bits/cow_string.h
> b/libstdc++-v3/include/bits/cow_string.h
> index 740e046f219..d5b39799254 100644
> --- a/libstdc++-v3/include/bits/cow_string.h
> +++ b/libstdc++-v3/include/bits/cow_string.h
> @@ -467,7 +467,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        _S_empty_rep() _GLIBCXX_NOEXCEPT
>        { return _Rep::_S_empty_rep(); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        // A helper type for avoiding boiler-plate.
>        typedef basic_string_view<_CharT, _Traits> __sv_type;
>
> @@ -685,7 +685,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         : _M_dataplus(_S_construct(__beg, __end, __a), __a)
>         { }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Construct string from a substring of a string_view.
>         *  @param  __t   Source object convertible to string view.
> @@ -775,7 +775,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        }
>  #endif // C++11
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Set value to string constructed from a string_view.
>         *  @param  __svt An object convertible to  string_view.
> @@ -1246,7 +1246,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        { return this->append(__l.begin(), __l.size()); }
>  #endif // C++11
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Append a string_view.
>         *  @param __svt The object convertible to string_view to be
> appended.
> @@ -1338,7 +1338,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         append(_InputIterator __first, _InputIterator __last)
>         { return this->replace(_M_iend(), _M_iend(), __first, __last); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Append a string_view.
>         *  @param __svt The object convertible to string_view to be
> appended.
> @@ -1496,7 +1496,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        { return this->assign(__l.begin(), __l.size()); }
>  #endif // C++11
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Set value from a string_view.
>         *  @param __svt The source object convertible to string_view.
> @@ -1703,7 +1703,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         return iterator(_M_data() + __pos);
>        }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Insert a string_view.
>         *  @param __pos  Position in string to insert at.
> @@ -2092,7 +2092,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        { return this->replace(__i1, __i2, __l.begin(), __l.end()); }
>  #endif // C++11
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Replace range of characters with string_view.
>         *  @param __pos  The position to replace at.
> @@ -2354,7 +2354,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        size_type
>        find(_CharT __c, size_type __pos = 0) const _GLIBCXX_NOEXCEPT;
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find position of a string_view.
>         *  @param __svt  The object convertible to string_view to locate.
> @@ -2432,7 +2432,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        size_type
>        rfind(_CharT __c, size_type __pos = npos) const _GLIBCXX_NOEXCEPT;
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find last position of a string_view.
>         *  @param __svt  The object convertible to string_view to locate.
> @@ -2515,7 +2515,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        find_first_of(_CharT __c, size_type __pos = 0) const
> _GLIBCXX_NOEXCEPT
>        { return this->find(__c, __pos); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find position of a character of a string_view.
>         *  @param __svt  An object convertible to string_view containing
> @@ -2599,7 +2599,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        find_last_of(_CharT __c, size_type __pos = npos) const
> _GLIBCXX_NOEXCEPT
>        { return this->rfind(__c, __pos); }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find last position of a character of string.
>         *  @param __svt  An object convertible to string_view containing
> @@ -2680,7 +2680,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        find_first_not_of(_CharT __c, size_type __pos = 0) const
>        _GLIBCXX_NOEXCEPT;
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find position of a character not in a string_view.
>         *  @param __svt  An object convertible to string_view containing
> @@ -2762,7 +2762,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        find_last_not_of(_CharT __c, size_type __pos = npos) const
>        _GLIBCXX_NOEXCEPT;
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Find last position of a character not in a string_view.
>         *  @param __svt  An object convertible to string_view containing
> @@ -2824,7 +2824,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         return __r;
>        }
>
> -#if __cplusplus >= 201703L
> +#ifdef __glibcxx_string_view // >= C++17
>        /**
>         *  @brief  Compare to a string_view.
>         *  @param __svt An object convertible to string_view to compare
> against.
> --
> 2.49.0
>
>

Reply via email to