[libcxx-commits] [PATCH] D68805: [libcxx] Remove shared_ptr::make_shared

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver marked 2 inline comments as done.
zoecarver added inline comments.


================
Comment at: libcxx/include/memory:3876
+    static shared_ptr<_Tp>
+    __create_with_cntrl_block(_Yp* __p, _CntrlBlk* __cntrl)
+    {
----------------
ldionne wrote:
> Can we call this `__create_with_control_block` instead? We're not saving many characters.
Will do.


================
Comment at: libcxx/include/memory:4421
+
+    return shared_ptr<_Tp>::__create_with_cntrl_block(__hold2.get()->get(),
+                                                      __hold2.release());
----------------
ldionne wrote:
> This just jumped in my face -- it's obvious now.
> 
> We're calling a function `f(p.get()->foobar(), p.release())` where `p` is a `unique_ptr`. If the second argument (`p.release()`) is evaluated before the first one, `p.get()` will be `nullptr` and we'll dereference that, causing the segfault. @phosek must have been testing on a target where the compiler evaluates arguments from right to left.
> 
> I think we pinned down the order of evaluation in C++20 (I'm not sure whether it actually got in), but it's definitely a problem for earlier standards anyway.
That would explain it! I'll change this to make `__hold2.get()->get()` a variable. 

I am surprised that compilers are allowed to mess with order of evaluation. Glad it's fixed in C++20, though. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68805