[2/2] libstdc++: More user-friendly failed assertions from shared_ptr dereference

  Currently dereferencing an empty shared_ptr prints a complicated
internal type in the assertion message:

include/bits/shared_ptr_base.h:1377: std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type& std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::operator*() const [with _Tp = std::filesystem::__cxx11::recursive_directory_iterator::_Dir_stack; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false; element_type = std::filesystem::__cxx11::recursive_directory_iterator::_Dir_stack]: Assertion '_M_get() != nullptr' failed.

Users don't care about any of the _Lp and <anonymous> template
parameters, so this is unnecessarily verbose.

We can simplify it to something that only mentions "shared_ptr_deref"
and the element type:

include/bits/shared_ptr_base.h:1371: _Tp* std::__shared_ptr_deref(_Tp*) [with _Tp = filesystem::__cxx11::recursive_directory_iterator::_Dir_stack]: Assertion '__p != nullptr' failed.

libstdc++-v3/ChangeLog:

	* include/bits/shared_ptr_base.h (__shared_ptr_deref): New
	function template.
	(__shared_ptr_access, __shared_ptr_access<>): Use it.
---
Tested x86_64-linux.

Also available for review at:
https://forge.sourceware.org/gcc/gcc-TEST/pulls/13

 libstdc++-v3/include/bits/shared_ptr_base.h | 28 +++++++++++++--------
 1 file changed, 17 insertions(+), 11 deletions(-)
  

Pushed

On Thu, 31 Oct 2024 at 20:08, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> Currently dereferencing an empty shared_ptr prints a complicated
> internal type in the assertion message:
>
> include/bits/shared_ptr_base.h:1377: std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::element_type& std::__shared_ptr_access<_Tp, _Lp, <anonymous>, <anonymous> >::operator*() const [with _Tp = std::filesystem::__cxx11::recursive_directory_iterator::_Dir_stack; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic; bool <anonymous> = false; bool <anonymous> = false; element_type = std::filesystem::__cxx11::recursive_directory_iterator::_Dir_stack]: Assertion '_M_get() != nullptr' failed.
>
> Users don't care about any of the _Lp and <anonymous> template
> parameters, so this is unnecessarily verbose.
>
> We can simplify it to something that only mentions "shared_ptr_deref"
> and the element type:
>
> include/bits/shared_ptr_base.h:1371: _Tp* std::__shared_ptr_deref(_Tp*) [with _Tp = filesystem::__cxx11::recursive_directory_iterator::_Dir_stack]: Assertion '__p != nullptr' failed.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/shared_ptr_base.h (__shared_ptr_deref): New
>         function template.
>         (__shared_ptr_access, __shared_ptr_access<>): Use it.
> ---
> Tested x86_64-linux.
>
> Also available for review at:
> https://forge.sourceware.org/gcc/gcc-TEST/pulls/13
>
>  libstdc++-v3/include/bits/shared_ptr_base.h | 28 +++++++++++++--------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h
> index 9a7617e7014..ee01594ce0c 100644
> --- a/libstdc++-v3/include/bits/shared_ptr_base.h
> +++ b/libstdc++-v3/include/bits/shared_ptr_base.h
> @@ -1337,6 +1337,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      { };
>
>
> +  template<typename _Tp>
> +    [[__gnu__::__always_inline__]]
> +    inline _Tp*
> +    __shared_ptr_deref(_Tp* __p)
> +    {
> +      __glibcxx_assert(__p != nullptr);
> +      return __p;
> +    }
> +
>    // Define operator* and operator-> for shared_ptr<T>.
>    template<typename _Tp, _Lock_policy _Lp,
>            bool = is_array<_Tp>::value, bool = is_void<_Tp>::value>
> @@ -1347,10 +1356,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>        element_type&
>        operator*() const noexcept
> -      {
> -       __glibcxx_assert(_M_get() != nullptr);
> -       return *_M_get();
> -      }
> +      { return *std::__shared_ptr_deref(_M_get()); }
>
>        element_type*
>        operator->() const noexcept
> @@ -1392,10 +1398,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        [[__deprecated__("shared_ptr<T[]>::operator* is absent from C++17")]]
>        element_type&
>        operator*() const noexcept
> -      {
> -       __glibcxx_assert(_M_get() != nullptr);
> -       return *_M_get();
> -      }
> +      { return *std::__shared_ptr_deref(_M_get()); }
>
>        [[__deprecated__("shared_ptr<T[]>::operator-> is absent from C++17")]]
>        element_type*
> @@ -1406,13 +1409,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        }
>  #endif
>
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wc++17-extensions"
>        element_type&
>        operator[](ptrdiff_t __i) const noexcept
>        {
> -       __glibcxx_assert(_M_get() != nullptr);
> -       __glibcxx_assert(!extent<_Tp>::value || __i < extent<_Tp>::value);
> -       return _M_get()[__i];
> +       if constexpr (extent<_Tp>::value)
> +         __glibcxx_assert(__i < extent<_Tp>::value);
> +       return std::__shared_ptr_deref(_M_get())[__i];
>        }
> +#pragma GCC diagnostic pop
>
>      private:
>        element_type*
> --
> 2.47.0
>