js8544 commented on issue #33628:
URL: https://github.com/apache/arrow/issues/33628#issuecomment-1428276965

   > `Scalar` has a problematic `const_cast` that makes any caller of 
`GetSharedPtr` a potential violator of const-correctness.
   > 
   > ```c++
   >   /// \brief EXPERIMENTAL Enable obtaining shared_ptr<Scalar> from a const
   >   /// Scalar& context.
   >   std::shared_ptr<Scalar> GetSharedPtr() const {
   >     return const_cast<Scalar*>(this)->shared_from_this();
   >   }
   > ```
   > 
   > `std::enable_shared_from_this` shows how to do this properly:
   > 
   > ```c++
   >       shared_ptr<_Tp>
   >       shared_from_this()
   >       { return shared_ptr<_Tp>(this->_M_weak_this); }
   > 
   >       shared_ptr<const _Tp>
   >       shared_from_this() const
   >       { return shared_ptr<const _Tp>(this->_M_weak_this); }
   > ```
   
   I think this `const_cast` is justifiable though, since the const version of  
`shared_from_this` returns `shared_ptr<const Scalar> `. To fix this we would 
need to write something like a `arrow::util::enable_shared_from_this` that 
returns `shared_ptr<_Tp>` for const _Tp. Do you think it's worth the hassle? cc 
@felipecrv @pitrou 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to