Hi, here is updated patch. I am not ceratin if: const size_t __diffmax = __gnu_cxx::__numeric_traits<ptrdiff_t>::__max / sizeof(_CharT); really needs "/ sizeof (_CharT)". I think we only need to be able to compute the difference between two entries in multiplies of sizeof(_CharT)? However it is consistent with what std::vector does. Do we really need to be also able to represent differences in number of bytes?
I was also thinking about API change implication. I hope that indeed real code does not need knowing max size and it is mostly needed to let GCC to VRP optimize the memory allocations. I also noticed that this patch trigger empty-loop.C failure which I originaly attributed to different change. I filled PR117764 on that. We are no longer able to eliminate empty loops early, but we still optimize them late. I also changed empty() to do the test directly (as done by std::vector and friends) instead of computing size. I do not think having __builtin_unreachable range check is useful there, so this should save some compile-time effort. Bootstrapped/regtested x86_64-linux, OK? libstdc++-v3/ChangeLog: * include/bits/basic_string.h (basic_string::size(), basic_string::length(), basic_string::capacity()): Add __builtin_unreachable to declare value ranges. (basic_string::empty): Inline test. (basic_string::max_size()): Account correctly the terminating 0 and limits implied by ptrdiff_t. gcc/testsuite/ChangeLog: * g++.dg/tree-ssa/empty-loop.C: xfail optimization at cddce2 and check it happens at cddce3. * g++.dg/tree-ssa/string-1.C: New test. diff --git a/gcc/testsuite/g++.dg/tree-ssa/empty-loop.C b/gcc/testsuite/g++.dg/tree-ssa/empty-loop.C index ed4a603bf5b..b7e7e27cc04 100644 --- a/gcc/testsuite/g++.dg/tree-ssa/empty-loop.C +++ b/gcc/testsuite/g++.dg/tree-ssa/empty-loop.C @@ -30,5 +30,8 @@ int foo (vector<string> &v, list<string> &l, set<string> &s, map<int, string> &m return 0; } -/* { dg-final { scan-tree-dump-not "if" "cddce2"} } */ +/* Adding __builtin_unreachable to std::string::size() prevents cddce2 from + eliminating the loop early, see PR117764. */ +/* { dg-final { scan-tree-dump-not "if" "cddce2" { xfail *-*-* } } } */ +/* { dg-final { scan-tree-dump-not "if" "cddce3"} } */ diff --git a/gcc/testsuite/g++.dg/tree-ssa/string-1.C b/gcc/testsuite/g++.dg/tree-ssa/string-1.C new file mode 100644 index 00000000000..d38c23a7628 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/string-1.C @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O3 -std=c++20 -fdump-tree-optimized" } */ +#include <string> +std::string +test (std::string &a) +{ + return a; +} +/* { dg-final { scan-tree-dump-not "throw" "optimized" } } */ diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index f5b320099b1..17b973c8b45 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -1079,20 +1079,30 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR size_type size() const _GLIBCXX_NOEXCEPT - { return _M_string_length; } + { + size_type __sz = _M_string_length; + if (__sz > max_size ()) + __builtin_unreachable (); + return __sz; + } /// Returns the number of characters in the string, not including any /// null-termination. _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR size_type length() const _GLIBCXX_NOEXCEPT - { return _M_string_length; } + { return size(); } /// Returns the size() of the largest possible %string. _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR size_type max_size() const _GLIBCXX_NOEXCEPT - { return (_Alloc_traits::max_size(_M_get_allocator()) - 1) / 2; } + { + const size_t __diffmax + = __gnu_cxx::__numeric_traits<ptrdiff_t>::__max / sizeof(_CharT); + const size_t __allocmax = _Alloc_traits::max_size(_M_get_allocator()); + return (std::min)(__diffmax, __allocmax) - 1; + } /** * @brief Resizes the %string to the specified number of characters. @@ -1184,8 +1194,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 size_type capacity() const _GLIBCXX_NOEXCEPT { - return _M_is_local() ? size_type(_S_local_capacity) - : _M_allocated_capacity; + size_t __sz = _M_is_local() ? size_type(_S_local_capacity) + : _M_allocated_capacity; + if (__sz < _S_local_capacity || __sz > max_size ()) + __builtin_unreachable (); + return __sz; } /** @@ -1234,7 +1247,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR bool empty() const _GLIBCXX_NOEXCEPT - { return this->size() == 0; } + { return _M_string_length == 0; } // Element access: /**