[libcxx-commits] [PATCH] D81417: [libcxx] Fix LWG 2874: Constructor shared_ptr::shared_ptr(Y*) should be constrained.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__memory/shared_ptr.h:439
+    template<class _Yp, class = _EnableIf<
+        _And<
+            __compatible_with<_Yp, _Tp>
----------------
zoecarver wrote:
> Another nit, feel free to take it or leave it: I don't think we need to introduce `_And` and `_If` here. Why don't we just do:
> ```
> _EnableIf<
>   __compatible_with<_Yp, _Tp>::value 
> #if
>   && (is_array<_Tp>::value ? 
>         __is_array_deletable<_Yp*>::value : 
>         __is_deletable<_Yp*>::value)
> #endif
> >
> ```
The problem here is that we only want to instantiate `__is_array_deletable<_Yp*>` or `__is_deletable<_Yp*>` when `is_array<_Tp>` is `true`/`false`, respectively. With your approach, we do it eagerly.

Similarly, we could do this:

```
__compatible_with<_Yp, _Tp>::value
#ifndef _LIBCPP_CXX03_LANG
  && _If<is_array<_Tp>::value, __is_array_deletable<_Yp*>, __is_deletable<_Yp*> >::value
#endif
```

But then, if the types are not compatible, we'll still be instantiating one check for `__is_array_deletable` or `__is_deletable`, depending on whether the type is an array.

The current approach delays and short cirtcuits the instantiation of SFINAE helpers as long as is possible. The only thing we do check eagerly is `is_array<_Tp>` regardless of the result of `__compatible_with`.

Furthermore, it used to be the case that we'd want to minimize these sorts of control structures in TMP code because they were heavier. However, nowadays we're using aliases to implement them, and some folks like Odin Holmes have demonstrated that they are very cheap.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81417/new/

https://reviews.llvm.org/D81417