[GSoC] Extend shared_ptr to support arrays
Fan You
youfan.noey@gmail.com
Thu Mar 19 06:21:00 GMT 2015
Thanks for pointing out inappropriate/wrong part in my code. After a
bit more reading and researching, I tried again,
starting from implementing first constructor in 8.2.1.1
<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4335.html#memory.smartptr.shared.const>.
However, there are still lots of parts I haven't working deeply, e.g.
~__shared_ptr() can't be just virtual and __shared_count haven't be
added.
Just to see if I get the right working procedure and I will continue
working on it.
2015-03-17 10:55 GMT-04:00 Jonathan Wakely <jwakely@redhat.com>:
> 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.
>
>
-------------- next part --------------
diff --git a/experimental/memory b/experimental/memory
new file mode 100644
index 0000000..8c51fc0
--- /dev/null
+++ b/experimental/memory
@@ -0,0 +1,20 @@
+#include <memory>
+
+namespace std {
+namespace experimental {
+inline namespace fundamentals_v1 {
+ template<typename _Tp>
+ class shared_ptr : __shared_ptr<_Tp>
+ {
+ public:
+ constexpr shared_ptr() noexcept = default;
+
+ template<typename _Tp1>
+ shared_ptr(_Tp1* __p)
+ : __shared_ptr<_Tp>(__p) { }
+
+ ~shared_ptr() = default;
+ };
+}
+}
+}
-------------- next part --------------
diff --git a/bits/shared_ptr_base.h b/bits/shared_ptr_base.h
index 026c597..41fb20b 100644
--- a/bits/shared_ptr_base.h
+++ b/bits/shared_ptr_base.h
@@ -1,3 +1,4 @@
+#include <iostream>
// shared_ptr and weak_ptr implementation details -*- C++ -*-
// Copyright (C) 2007-2014 Free Software Foundation, Inc.
@@ -1175,6 +1176,39 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__shared_count<_Lp> _M_refcount; // Reference counter.
};
+ //array support
+ template<typename _Tp>
+ class __shared_ptr<_Tp[]>
+ {
+ public:
+ __shared_ptr() = default;
+ template<typename _Tp1>
+ __shared_ptr(_Tp1* __p) : _M_ptr(__p)
+ {
+ __glibcxx_function_requires(_ConvertibleConcept<_Tp1*, _Tp*>)
+ static_assert( !is_void<_Tp1>::value, "incomplete type" );
+ static_assert( sizeof(_Tp1) > 0, "incomplete type" );
+ //TODO enable shared
+ std::cout << "array shared_ptr basic constructor" << "\n";
+ }
+ virtual
+ ~__shared_ptr()
+ { _M_del(_M_ptr); }
+ protected:
+ struct _Deleter
+ {
+ void
+ operator()(_Tp const *__p)
+ {
+ delete [] __p;
+ std::cout << "array shared_ptr default destructor" << "\n";
+ }
+ };
+ private:
+ _Tp* _M_ptr;
+ _Deleter _M_del; //default destructor
+ };
+
// 20.7.2.2.7 shared_ptr comparisons
template<typename _Tp1, typename _Tp2, _Lock_policy _Lp>
More information about the Libstdc++
mailing list