Merge pull request #113 from Point72/tkp/fixes

Accumulated Fixes - #80, #74

cpp/csp/core/Exception.h

@@ -27,7 +27,7 @@ class Exception : public std::exception
  m_full = m_file + ":" + m_function + ":" + std::to_string( m_line ) + ":";
  m_full += m_exType + ": " + m_description;
  if( includeBacktrace && m_backtracesize > 0 )
- m_full += "\n" + backtraceString();
+ m_full += '\n' + backtraceString();
  return m_full;
  }
  const std::string & description() const noexcept { return m_description; }

cpp/csp/engine/Struct.cpp

@@ -5,7 +5,7 @@
 namespace csp
 {
 
-StructField::StructField( CspTypePtr type, const std::string & fieldname, 
+StructField::StructField( CspTypePtr type, const std::string & fieldname,
  size_t size, size_t alignment ) :
  m_fieldname( fieldname ),
  m_offset( 0 ),
@@ -18,15 +18,15 @@ StructField::StructField( CspTypePtr type, const std::string & fieldname,
 {
 }
 
-/* StructMeta 
+/* StructMeta
 
 A note on member layout. Meta will order objects in the following order:
 - non-native fields ( ie PyObjectPtr for the python dialect )
 - native fields sorted in order
 - set/unset bitmask bytes ( 1 byte per 8 fields )
 
 Derived structs will simply append to the layout of the base struct, properly padding between classes to align
-its fields properly. 
+its fields properly.
 This layout is imposed on Struct instances. Since Struct needs refcount and meta * fields, for convenience they are stored
 *before* a Struct's "this" pointer as hidden data. This way struct ptrs can be passed into StructMeta without
 and adjustments required for the hidden fields
@@ -47,7 +47,7 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields,
  //decided to place them at the start cause they are most likely size of ptr or greater
 
  m_fieldnames.reserve( m_fields.size() );
- for( size_t i = 0; i < m_fields.size(); i++ ) 
+ for( size_t i = 0; i < m_fields.size(); i++ )
  m_fieldnames.emplace_back( m_fields[i] -> fieldname() );
 
  std::sort( m_fields.begin(), m_fields.end(), []( auto && a, auto && b )
@@ -93,7 +93,7 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields,
  m_isFullyNative = m_isPartialNative && ( m_base ? m_base -> isNative() : true );
 
  //Setup masking bits for our fields
- //NOTE we can be more efficient by sticking masks into any potential alignment gaps, dont want to spend time on it 
+ //NOTE we can be more efficient by sticking masks into any potential alignment gaps, dont want to spend time on it
  //at this point
  m_maskSize = !m_fields.empty() ? 1 + ( ( m_fields.size() - 1 ) / 8 ) : 0;
  m_size = offset + m_maskSize;
@@ -116,7 +116,7 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields,
  {
  m_fields.insert( m_fields.begin(), m_base -> m_fields.begin(), m_base -> m_fields.end() );
  m_fieldnames.insert( m_fieldnames.begin(), m_base -> m_fieldnames.begin(), m_base -> m_fieldnames.end() );
- 
+
  m_firstPartialField = m_base -> m_fields.size();
  m_firstNativePartialField += m_base -> m_fields.size();
  m_fieldMap = m_base -> m_fieldMap;
@@ -138,9 +138,9 @@ StructMeta::~StructMeta()
 Struct * StructMeta::createRaw() const
 {
  Struct * s;
- //This weird looking new call actually is calling Struct::opereatore new( size_t, StructMeta * )
+ //This weird looking new call actually is calling Struct::opereatore new( size_t, std::shared_ptr<StructMeta> )
  //its not actually doing an in-place on StructMeta this
- s = new ( this ) Struct( this );
+ s = new ( shared_from_this() ) Struct( shared_from_this() );
 
  initialize( s );
 
@@ -212,7 +212,7 @@ void StructMeta::initialize( Struct * s ) const
  }
 
  memset( reinterpret_cast<std::byte*>(s) + m_nativeStart, 0, partialNativeSize() );
- 
+
  if( !m_isPartialNative )
  {
  for( size_t idx = m_firstPartialField; idx < m_firstNativePartialField; ++idx )
@@ -221,7 +221,7 @@ void StructMeta::initialize( Struct * s ) const
  static_cast<NonNativeStructField*>( field ) -> initialize( s );
  }
  }
- 
+
  if( m_base )
  m_base -> initialize( s );
 }
@@ -231,13 +231,13 @@ void StructMeta::copyFrom( const Struct * src, Struct * dest )
  if( unlikely( src == dest ) )
  return;
 
- if( dest -> meta() != src -> meta() && 
+ if( dest -> meta() != src -> meta() &&
  !StructMeta::isDerivedType( src -> meta(), dest -> meta() ) )
- CSP_THROW( TypeError, "Attempting to copy from struct type '" << src -> meta() -> name() << "' to struct type '" << dest -> meta() -> name() 
+ CSP_THROW( TypeError, "Attempting to copy from struct type '" << src -> meta() -> name() << "' to struct type '" << dest -> meta() -> name()
  << "'. copy_from may only be used to copy from same type or derived types" );
 
  dest -> meta() -> copyFromImpl( src, dest );
-} 
+}
 
 void StructMeta::copyFromImpl( const Struct * src, Struct * dest ) const
 {
@@ -264,7 +264,7 @@ void StructMeta::copyFromImpl( const Struct * src, Struct * dest ) const
  //note that partialNative will include the mask bytes - this sets the native part and the mask
  memcpy( reinterpret_cast<std::byte*>(dest) + m_nativeStart, reinterpret_cast<const std::byte*>(src) + m_nativeStart,
  partialNativeSize() );
- 
+
  if( m_base )
  m_base -> copyFromImpl( src, dest );
  }
@@ -275,13 +275,13 @@ void StructMeta::updateFrom( const Struct * src, Struct * dest )
  if( unlikely( src == dest ) )
  return;
 
- if( dest -> meta() != src -> meta() && 
+ if( dest -> meta() != src -> meta() &&
  !StructMeta::isDerivedType( src -> meta(), dest -> meta() ) )
- CSP_THROW( TypeError, "Attempting to update from struct type '" << src -> meta() -> name() << "' to struct type '" << dest -> meta() -> name() 
+ CSP_THROW( TypeError, "Attempting to update from struct type '" << src -> meta() -> name() << "' to struct type '" << dest -> meta() -> name()
  << "'. update_from may only be used to update from same type or derived types" );
 
  dest -> meta() -> updateFromImpl( src, dest );
-} 
+}
 
 void StructMeta::updateFromImpl( const Struct * src, Struct * dest ) const
 {
@@ -312,7 +312,7 @@ bool StructMeta::isEqual( const Struct * x, const Struct * y ) const
 
  //Note the curent use of memcpy for native types. This can cause issues on double comparisons
  //esp if expecting NaN == NaN to be false, and when comparing -0.0 to +0.0.. may want to revisit
- //We we do we may as well remove the basepadding copy 
+ //We we do we may as well remove the basepadding copy
  if( isNative() )
  return memcmp( x, y, size() ) == 0;
 
@@ -325,7 +325,7 @@ bool StructMeta::isEqual( const Struct * x, const Struct * y ) const
  for( size_t idx = m_firstPartialField; idx < m_firstNativePartialField; ++idx )
  {
  auto * field = m_fields[ idx ].get();
- 
+
  if( field -> isSet( x ) != field -> isSet( y ) )
  return false;
 
@@ -370,19 +370,19 @@ size_t StructMeta::hash( const Struct * x ) const
  "Exceeded max recursion depth of " << MAX_RECURSION_DEPTH << " in " << name() << "::hash(), cannot hash cyclic data structure" );
 
  hash ^= csp::hash::hash_bytes( x + m_nativeStart, partialNativeSize() );
- 
+
  if( !m_isPartialNative )
  {
  for( size_t idx = m_firstPartialField; idx < m_firstNativePartialField; ++idx )
  {
  auto * field = m_fields[ idx ].get();
- 
+
  //we dont incorporate unset fields, bitmask will cover them
  if( field -> isSet( x ) )
  hash ^= static_cast<NonNativeStructField*>( field ) -> hash( x );
  }
  }
- 
+
  if( m_base )
  hash ^= std::hash<uint64_t>()( (uint64_t ) m_base.get() ) ^ m_base -> hash( x );
 
@@ -398,18 +398,18 @@ void StructMeta::clear( Struct * s ) const
  }
 
  memset( reinterpret_cast<std::byte*>(s) + m_nativeStart, 0, partialNativeSize() );
- 
+
  if( !m_isPartialNative )
  {
  for( size_t idx = m_firstPartialField; idx < m_firstNativePartialField; ++idx )
  {
  auto * field = m_fields[ idx ].get();
- 
+
  if( field -> isSet( s ) )
  static_cast<NonNativeStructField*>( field ) -> clearValue( s );
  }
  }
- 
+
  if( m_base )
  m_base -> clear( s );
 }
@@ -450,19 +450,22 @@ void StructMeta::destroy( Struct * s ) const
  static_cast<NonNativeStructField*>( field ) -> destroy( s );
  }
  }
- 
+
  if( m_base )
  m_base -> destroy( s );
 }
 
-Struct::Struct( const StructMeta * meta )
+Struct::Struct( const std::shared_ptr<const StructMeta> & meta )
 {
+ //Initialize meta shared_ptr
+ new( hidden() ) HiddenData();
+
  hidden() -> refcount = 1;
  hidden() -> meta = meta;
  hidden() -> dialectPtr = nullptr;
 }
 
-void * Struct::operator new( std::size_t count, const StructMeta * meta )
+void * Struct::operator new( std::size_t count, const std::shared_ptr<const StructMeta> & meta )
 {
  //allocate meta -> size() ( data members ) + room for hidden data refcount and meta ptr
  //Note that we currently assume sizeof( HiddenData ) is 16. We expect it to be a multiple of 8

cpp/csp/engine/Struct.h

@@ -21,7 +21,7 @@ using StructPtr = TypedStructPtr<Struct>;
 class StructField
 {
 public:
- 
+
  virtual ~StructField() {}
 
  const std::string & fieldname() const { return m_fieldname; }
@@ -31,7 +31,7 @@ class StructField
  size_t alignment() const { return m_alignment; } //alignment of the field
  size_t maskOffset() const { return m_maskOffset; } //offset to location of the mask byte fo this field from start of struct mem
  uint8_t maskBit() const { return m_maskBit; } //bit within mask byte associated with this field
- uint8_t maskBitMask() const { return m_maskBitMask; } //same as maskBit but as a mask ( 1 << bit 
+ uint8_t maskBitMask() const { return m_maskBitMask; } //same as maskBit but as a mask ( 1 << bit
 
  bool isNative() const { return m_type -> type() <= CspType::Type::MAX_NATIVE_TYPE; }
 
@@ -41,8 +41,8 @@ class StructField
  CSP_ASSERT( bit < 8 );
 
  m_maskOffset = off;
- m_maskBit = bit; 
- m_maskBitMask = 1 << bit; 
+ m_maskBit = bit;
+ m_maskBitMask = 1 << bit;
  }
 
  bool isSet( const Struct * s ) const
@@ -72,22 +72,22 @@ class StructField
 
 protected:
 
- StructField( CspTypePtr type, const std::string & fieldname, 
+ StructField( CspTypePtr type, const std::string & fieldname,
  size_t size, size_t alignment );
 
  void setIsSet( Struct * s ) const
  {
  uint8_t * m = reinterpret_cast<uint8_t *>( s ) + m_maskOffset;
  (*m) |= m_maskBitMask;
  }
- 
+
  const void * valuePtr( const Struct * s ) const
- { 
+ {
  return valuePtr( const_cast<Struct*>( s ) );
  }
 
  void * valuePtr( Struct * s ) const
- { 
+ {
  return reinterpret_cast<uint8_t *>( s ) + m_offset;
  }
 
@@ -96,7 +96,7 @@ class StructField
  uint8_t * m = reinterpret_cast<uint8_t *>( s ) + m_maskOffset;
  (*m) &= ~m_maskBitMask;
  }
- 
+
 private:
  std::string m_fieldname;
  size_t m_offset;
@@ -507,7 +507,7 @@ class TypedStructPtr
  void incref();
 
  T * m_obj;
- 
+
  template<typename U>
  friend class TypedStructPtr;
 
@@ -524,7 +524,7 @@ TypedStructPtr<T> structptr_cast( const TypedStructPtr<U> & r )
  return out;
 }
 
-class StructMeta
+class StructMeta : public std::enable_shared_from_this<StructMeta>
 {
 public:
  using Fields = std::vector<StructFieldPtr>;
@@ -539,18 +539,18 @@ class StructMeta
  size_t partialSize() const { return m_partialSize; }
 
  bool isNative() const { return m_isFullyNative; }
- 
+
  const Fields & fields() const { return m_fields; }
  const FieldNames & fieldNames() const { return m_fieldnames; }
 
  size_t maskLoc() const { return m_maskLoc; }
  size_t maskSize() const { return m_maskSize; }
 
  const StructFieldPtr & field( const char * name ) const
- { 
+ {
  static StructFieldPtr s_empty;
- auto it = m_fieldMap.find( name ); 
- return it == m_fieldMap.end() ? s_empty : it -> second; 
+ auto it = m_fieldMap.find( name );
+ return it == m_fieldMap.end() ? s_empty : it -> second;
  }
 
  const StructFieldPtr & field( const std::string & name ) const
@@ -631,7 +631,7 @@ class Struct
 {
 public:
 
- const StructMeta * meta() const { return hidden() -> meta; }
+ const StructMeta * meta() const { return hidden() -> meta.get(); }
  size_t refcount() const { return hidden() -> refcount; }
 
  bool operator==( const Struct & rhs ) const { return meta() -> isEqual( this, &rhs ); }
@@ -673,10 +673,10 @@ class Struct
  void setDialectPtr( void * p ) { hidden() -> dialectPtr = p; }
 
 private:
- static void * operator new( std::size_t count, const StructMeta * meta );
+ static void * operator new( std::size_t count, const std::shared_ptr<const StructMeta> & meta );
  static void operator delete( void * ptr );
 
- Struct( const StructMeta * meta );
+ Struct( const std::shared_ptr<const StructMeta> & meta );
  ~Struct()
  {
  meta() -> destroy( this );
@@ -703,7 +703,7 @@ class Struct
  struct HiddenData
  {
  size_t refcount;
- const StructMeta * meta;
+ std::shared_ptr<const StructMeta> meta;
  void * dialectPtr;
  };
 
@@ -730,7 +730,7 @@ template<typename T>
 inline void TypedStructPtr<T>::decref()
 {
  m_obj -> decref();
-} 
+}
 
 
 template<typename T>
@@ -746,7 +746,7 @@ bool TypedStructPtr<T>::operator==( const TypedStructPtr<T> & rhs ) const
 class StructStructField final : public NonNativeStructField
 {
 public:
- StructStructField( CspTypePtr cspType, const std::string &fieldname ) : 
+ StructStructField( CspTypePtr cspType, const std::string &fieldname ) :
  NonNativeStructField( cspType, fieldname, sizeof( StructPtr ), alignof( StructPtr ) )
  {
  CSP_ASSERT( cspType -> type() == CspType::Type::STRUCT );