On Wed, Jun 24, 2015 at 10:33 AM, Fan You <youfan.n...@gmail.com> wrote:
> Hi,
>
> Here is the revised patch including all the test case.
>
> This can also be seen at <https://github.com/Noeyfan/gcc-1> on branch
> <shared_arrays>
>
> Any comments?

I ran `git diff c7248656569bb0b4549f5c1ed347f7e028a15664
90aff5632fd9f3044d53ce190ae99fb69c41ce49`.

To systematically detect consecutive spaces (to convert them to tabs),
I'll just simply do:
`egrep "^\t* {8}" shared_ptr*`

-       = typename conditional<is_array<_Tp>::value, _Array_Deleter,
_Normal_Deleter>::type;
+       = typename conditional<is_array<_Tp>
+                               ::value, _Array_Deleter, _Normal_Deleter>::type;
Tabs. Also, I personally prefer to put '::value' to the same line as
is_array<_Tp>.

-      using __base_type = __shared_ptr<element_type>;
+      using __Base_type = __shared_ptr<element_type>;
_Base_type, not __Base_type. Also, the mostly used is _Base:

...src/gcc/libstdc++-v3 % grep -r '_Base[a-zA-Z_0-9]*' . -o | grep
':.*$' -o|sort|uniq -c
   2350 :_Base
      1 :_Base_biteset
     62 :_Base_bitset
    120 :_Base_const_iterator
     20 :_Base_const_local_iterator
      4 :_Based
    177 :_Base_iterator
      1 :_Base_Iterator
      8 :_Base_local_iterator
     21 :_Base_manager
    133 :_Base_ptr
      9 :_Base_ref
      2 :_BaseSequence
    173 :_Base_type
      3 :_BaseType

-       : __base_type(__p, _Deleter_type())
+       : __Base_type(__p, _Deleter_type())
Please be aware of tabs.

-      template<typename _Tp1, typename = _Convertible<_Tp1*>>
+      template<typename _Tp1, typename = _Compatible<_Tp1>>
        __shared_ptr(__shared_ptr<__libfund_v1<_Tp1>, _Lp>&& __r) noexcept
-        : __base_type(std::move(__r))
+        : __Base_type(static_cast<typename __shared_ptr
+                      <__libfund_v1<_Tp1>>::__Base_type&&>(std::move(__r)))
static_cast<typename __shared_ptr<__libfund_v1<_Tp1>>::__Base_type&&>(__r)
is enough, since std::move is actually a static_cast to rvalue reference.

Alternatively, you may define a template alias for the static_cast, if
you find it too long.

-       operator=(const __shared_ptr<__libfund_v1<_Tp1>, _Lp>& __r) noexcept
+       operator=(const __shared_ptr<_Tp1, _Lp>& __r) noexcept
Why?

    template<typename _Tp>
      inline bool
      operator<(const shared_ptr<_Tp>& __a, nullptr_t) noexcept
-     {
+     {
        using _Tp_RE = typename remove_extent<_Tp>::type;
-       return std::less<_Tp_RE>()(__a.get(), nullptr);
+       return std::less<_Tp_RE>()(__a.get(), nullptr);
      }
using _Tp_RE = typename shared_ptr<_Tp>::element_type;



-- 
Regards,
Tim Shen

Reply via email to