[GSoC] Extend shared_ptr to support arrays

Jonathan Wakely jwakely@redhat.com
Tue Mar 17 14:55:00 GMT 2015


On 16/03/15 15:14 -0400, Fan You wrote:
>Hello,
>
>It's my simple version of extending shared_ptr to support arrays, am I
>in the right direction?
>
>When I do the boundary checking, when user try to access elements that
>are out of boundary, should I throw an exception or just use assert to
>avoid that from happening? (I saw boost use BOOST_ASSERT to do that)
>
>Next, my plan for GSoC is to finish shared_ptr_array_support first and
>then choose one or two from other TS.
>
>For those TS I am looking into:
>
>- Invocation type traits
>  I have a very limited compiler knowledge, so I am still learning it.
>
>- Polymorphic Memory Resources
>  After reading the proposal
><http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3916.pdf>, I
>am still not quite sure about the idea of Polymorphic memory
>resources. The example on page 5 make sense to me, but what's the key
>different between the original memory allocation method and this? Any
>other resource that I can read from?

The point is to allow different allocators to be used without becoming
part of a container's type.

>- Networking TS
>  I get the wrong link last time, but I find it myself(don't know if
>it's correct <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4370.html>).
>The proposal intend to implement a whole Network Library and it's
>currently too big for me to read them all. Can you suggest a specific
>part of it that I should start looking at or working on?

I already have a partial implementation. The hardest part is probably 
implementing thread pools or other executaion agents that can run the
asynchronous tasks. That would be easy to do badly, and hard to do
well.

>Is it OK and realistic to propose multiple TS in GSoC? Any suggestion
>or critic are welcome!

I don't think it's realistic to try and do the Networking TS as well
as the other pieces you've mentioned. Completing the missing parts of
the Fundamentals TS seems like a reasonable project on its own, and
the Networking TS is a reasonable project on its own.

Have you started the process of completing the necessary paperwork? We
will not be able to accept code without a copyright assignment, and we
won't accept a GSoC proposal that will produce code we can't use!

Some initial comments on the code ...


>diff --git a/bits/shared_ptr.h b/bits/shared_ptr.h
>index 081d3bd..d5f6e1c 100644
>--- a/bits/shared_ptr.h
>+++ b/bits/shared_ptr.h

Array support is defined in terms of a separate class template,
std::experimental::shared_ptr, which goes in a separate header, not
the main <bits/shared_ptr.h> file.

The correct document to work from is
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4335.html 

>@@ -614,9 +614,182 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       { return std::hash<_Tp*>()(__s.get()); }
>     };
>
>-  // @} group pointer_abstractions
>+	/// specialization for shared_ptr_array
>
>-_GLIBCXX_END_NAMESPACE_VERSION
>+	//dynamic size
>+	template<typename _Tp>
>+		class shared_ptr<_Tp[]> : public __shared_ptr<_Tp>{

I would expect array support to be added to __shared_ptr, instead of
only the derived shared_ptr type.

>+			public:

Your indentation is crazy :-)

Please look at the existing library code for the coding style, as well
as
https://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html

>+				constexpr shared_ptr<_Tp[]>() noexcept :

Please don't put the redunant template argument list here, it should
be just shared_ptr not shared_ptr<_Tp[]>. Again, look at the existing
code to see how we write libstdc++ code.

>+					__shared_ptr<_Tp>() { }

This constructor should be defined as defaulted (the only reason the
primary template doesn't do that is because it was written before G++
supported defaulted functions).

>+
>+				template<typename _Tp1>
>+					explicit shared_ptr<_Tp[]>(_Tp1* __p) :
>+						 __shared_ptr<_Tp>(__p, _A_Del) { }
>+
>+				template<typename _Tp1, typename _Deleter>
>+					shared_ptr<_Tp[]>(_Tp1* __p, _Deleter __d) :
>+						 __shared_ptr<_Tp>(__p, __d) { }
>+
>+				template<typename _Deleter>
>+					shared_ptr<_Tp[]>(nullptr_t __p, _Deleter __d) :
>+						 __shared_ptr<_Tp>(__p, __d) { }
>+
>+				template<typename _Tp1, typename _Deleter, typename _Alloc>
>+					shared_ptr<_Tp[]>(_Tp1* __p, _Deleter __d, _Alloc __a):
>+						__shared_ptr<_Tp>(__p, __d, std::move(__a)) { }
>+
>+				template<typename _Deleter, typename _Alloc>
>+					shared_ptr<_Tp[]>(nullptr_t __p, _Deleter __d, _Alloc __a):
>+						__shared_ptr<_Tp>(__p, __d, std::move(__a)) { }
>+
>+				template<typename _Tp1>
>+					shared_ptr<_Tp[]>(const shared_ptr<_Tp1[]>& __r, _Tp* __p) noexcept :
>+					__shared_ptr<_Tp>(__r, __p) { }
>+
>+				template<typename _Tp1, typename = typename
>+					std::enable_if<std::is_convertible<_Tp1*, _Tp*>::value>::type>

I think this constraint is wrong, it should only allow the addition of
cv-qualifiers, not arbitrary conversions. See the definition of
"compatible with" in the TS.

>+					shared_ptr<_Tp[]>(const shared_ptr<_Tp1[]>& __r) noexcept :
>+					__shared_ptr<_Tp>(__r) { }
>+
>+				shared_ptr<_Tp[]>(shared_ptr<_Tp[]>& __r) = default;
>+
>+				//shared_ptr<_Tp[]>(shared_ptr<_Tp[]>&& __r) noexcept :
>+				//	__shared_ptr<_Tp>(std::move(__r)) { }
>+
>+				template<typename _Tp1, typename = typename
>+					std::enable_if<std::is_convertible<_Tp1*, _Tp*>::value>::type>

Wrong constraint again.

>+					shared_ptr<_Tp[]>(shared_ptr<_Tp1[]>&& __r) noexcept
>+					: __shared_ptr<_Tp>(std::move(__r)) { }
>+
>+#if _GLIBCXX_USE_DEPRECATED
>+				template<typename _Tp1>
>+					shared_ptr<_Tp[]>(std::auto_ptr<_Tp1>&& __r); // TODO auto_ptr still calls delete p;

shared_ptr<T[]> does not support construction from auto_ptr.

>+#endif
>+
>+				template<typename _Tp1, typename _Del>
>+					shared_ptr<_Tp[]>(std::unique_ptr<_Tp1[], _Del>&& __r) :
>+					__shared_ptr<_Tp>(std::move(__r)) { }
>+
>+				constexpr shared_ptr<_Tp[]>(nullptr_t __p) noexcept :
>+					__shared_ptr<_Tp>(__p) { }
>+
>+#if _GLIBCXX_USE_DEPRECATED
>+				//TODO
>+#endif
>+
>+				shared_ptr<_Tp[]>& operator=(const shared_ptr<_Tp[]>&) noexcept = default;
>+
>+				template<typename _Tp1>
>+					shared_ptr<_Tp[]>&
>+					operator=(const shared_ptr<_Tp1[]>& __r) noexcept {
>+						this->__shared_ptr<_Tp>::operator=(__r);
>+						return *this;
>+					}
>+
>+				shared_ptr<_Tp[]>&
>+					operator=(shared_ptr<_Tp[]>&& __r) noexcept

This should be defaulted.

>+					{
>+						this->__shared_ptr<_Tp>::operator=(std::move(__r));
>+						return *this;
>+					}
>+
>+				template<typename _Tp1, typename _Del>
>+					shared_ptr<_Tp[]>&
>+					operator=(std::unique_ptr<_Tp1[], _Del>&& __r)
>+					{
>+						this->__shared_ptr<_Tp>::operator=(std::move(__r));
>+						return *this;
>+					}
>+
>+				// access element in array
>+				typename std::add_lvalue_reference<_Tp>::type
>+				operator[](size_t __i) const{
>+					return (this->get())[__i];
>+				}
>+
>+			protected:
>+				struct _Deleter {
>+					void operator()(_Tp const *p) {
>+						delete [] p;
>+					}
>+				};
>+
>+			private:
>+				_Deleter _A_Del;

It should not be necessary to have a deleter as a member of this
class, it is stored in the base class if required.




More information about the Libstdc++ mailing list