Skip to content

Allow derived structs to override base fields with subtypes #681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AdamGlustein merged 3 commits into from
Mar 16, 2026
Merged

Allow derived structs to override base fields with subtypes #681

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f718205
Allow derived structs to override base fields with subtypes
AdamGlustein Feb 23, 2026
170bb72
Try to simplify the changes by first adding base fields in the meta c…
AdamGlustein Mar 12, 2026
eaf9f4b
Use local derivedFields for offset/mask calculations and other meta p…
AdamGlustein Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 72 additions & 36 deletions cpp/csp/engine/Struct.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,80 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields, bool is

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
//at this point
if( m_base )
Comment thread
robambalu marked this conversation as resolved.
{
// The complete inheritance hierarchy must agree on strict/non-strict
if( m_isStrict != m_base -> isStrict() )
{
CSP_THROW( ValueError,
"Struct " << m_name << " was declared " << ( m_isStrict ? "strict" : "non-strict" ) << " but derives from "
<< m_base -> name() << " which is " << ( m_base -> isStrict() ? "strict" : "non-strict" )
);
}

size_t optionalFieldCount = std::count_if( m_fields.begin(), m_fields.end(), []( const auto & f ) { return f -> isOptional(); } );

m_maskSize = !m_fields.empty() ? 1 + ( ( m_fields.size() + optionalFieldCount - 1 ) / 8 ) : 0;
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;
}

size_t idx = m_firstPartialField;
while( idx < m_fields.size() )
{
auto fieldname = m_fields[ idx ] -> fieldname().c_str();
auto rv = m_fieldMap.emplace( fieldname, m_fields[ idx ] );
if( !rv.second )
{
auto derivedTypeEnum = m_fields[ idx ] -> type() -> type();
auto baseTypeEnum = rv.first -> second -> type() -> type();

// only allowed to override fields from a base class if types are DIALECT_GENERIC or STRUCT
// beyond that, its up to the dialect-specific struct impl (i.e. PyStruct) to decide whether the override types are valid
if( derivedTypeEnum != baseTypeEnum || ( derivedTypeEnum != CspType::Type::DIALECT_GENERIC && derivedTypeEnum != CspType::Type::STRUCT ) )
CSP_THROW( ValueError, "csp Struct " << name << " attempted to add existing field " << m_fields[ idx ] -> fieldname() <<
" which is not allowed: base field type " << baseTypeEnum << " and derived field type " << derivedTypeEnum );

// insert the derived override field into the space allocated for the base field
auto derivedField = m_fields[ idx ];
for( size_t baseIdx = 0; baseIdx < m_firstPartialField; ++baseIdx )
{
if( m_fields[ baseIdx ] -> fieldname() == derivedField -> fieldname() )
{
derivedField -> setOffset( m_fields[ baseIdx ] -> offset() );
derivedField -> setMaskOffset( m_fields[ baseIdx ] -> maskOffset(), m_fields[ baseIdx ] -> maskBit() );
m_fields[ baseIdx ] = derivedField;
break;
}
}
rv.first -> second = derivedField; // field map

// clean up the field we just removed on the derived meta
// the erased field had to be non-native (only DIALECT_GENERIC and STRUCT overrides are allowed)
// so adjust the non-native/native boundary index for the derived partial section
--m_firstNativePartialField;
m_isPartialNative = ( m_firstPartialField == m_firstNativePartialField );
m_fields.erase( m_fields.begin() + idx );
auto nameIt = std::find( m_fieldnames.begin() + m_firstPartialField, m_fieldnames.end(), derivedField -> fieldname() );
m_fieldnames.erase( nameIt );
}
else
++idx;
}

// setup masking bits for the remaining derived (partial) fields
// note that this is done after the override dedup above so that overridden fields don't consume mask bits.
//NOTE we can be more efficient by sticking masks into any potential alignment gaps, dont want to spend time on it
size_t optionalFieldCount = std::count_if( m_fields.begin() + m_firstPartialField, m_fields.end(), []( const auto & f ) { return f -> isOptional(); } );
size_t partialFieldCount = m_fields.size() - m_firstPartialField;

m_maskSize = partialFieldCount > 0 ? 1 + ( ( partialFieldCount + optionalFieldCount - 1 ) / 8 ) : 0;
m_size = offset + m_maskSize;
m_partialSize = m_size - baseSize;
m_maskLoc = m_size - m_maskSize;
uint8_t numRemainingBits = ( m_fields.size() + optionalFieldCount ) % 8;

uint8_t numRemainingBits = ( partialFieldCount + optionalFieldCount ) % 8;
m_lastByteMask = ( 1u << numRemainingBits ) - 1;

size_t maskLoc = m_maskLoc;
Expand All @@ -118,7 +180,7 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields, bool is
// Set optional fields first so that their 2-bits never cross a byte boundary
// Put both the set bits and none bits in the same vector to avoid fragmentation
m_optionalFieldsBitMasks.resize( 2 * m_maskSize );
for( size_t i = 0; i < m_fields.size(); ++i )
for( size_t i = m_firstPartialField; i < m_fields.size(); ++i )
{
auto & f = m_fields[ i ];
if( f -> isOptional() )
Expand All @@ -134,7 +196,7 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields, bool is
}
}

for( size_t i = 0; i < m_fields.size(); ++i )
for( size_t i = m_firstPartialField; i < m_fields.size(); ++i )
{
auto & f = m_fields[ i ];
if( !f -> isOptional() )
Expand All @@ -147,32 +209,6 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields, bool is
}
}
}

if( m_base )
{
// The complete inheritance hierarchy must agree on strict/non-strict
if( m_isStrict != m_base -> isStrict() )
{
CSP_THROW( ValueError,
"Struct " << m_name << " was declared " << ( m_isStrict ? "strict" : "non-strict" ) << " but derives from "
<< m_base -> name() << " which is " << ( m_base -> isStrict() ? "strict" : "non-strict" )
);
}

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;
}

for( size_t idx = m_firstPartialField; idx < m_fields.size(); ++idx )
{
auto rv = m_fieldMap.emplace( m_fields[ idx ] -> fieldname().c_str(), m_fields[ idx ] );
if( !rv.second )
CSP_THROW( ValueError, "csp Struct " << name << " attempted to add existing field " << m_fields[ idx ] -> fieldname() );
}
}

StructMeta::~StructMeta()
Expand Down
46 changes: 46 additions & 0 deletions cpp/csp/python/PyStruct.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,52 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj
metabase = ( ( PyStructMeta * ) base ) -> structMeta;
}
}

// validate overriden fields in the derived class, if they exist
if( metabase )
{
for( const auto & derivedField : fields )
{
const auto & baseField = metabase -> field( derivedField -> fieldname() );
if( !baseField )
continue;

// ensure the types are compatible i.e. derived field type is a subclass of base
auto baseTypeEnum = baseField -> type() -> type();
auto derivedTypeEnum = derivedField -> type() -> type();

if( baseTypeEnum != derivedTypeEnum )
{
CSP_THROW( TypeError, "Field '" << derivedField -> fieldname() << "' on struct " << name
<< " has type incompatible with base field type: base type is " << baseTypeEnum
<< ", derived type is " << derivedTypeEnum );
}

// field overrides are only valid for Python objects and structs, not native types
if( baseTypeEnum == CspType::Type::DIALECT_GENERIC )
{
auto * basePyField = static_cast<PyObjectStructField *>( baseField.get() );
auto * derivedPyField = static_cast<PyObjectStructField *>( derivedField.get() );
if( !PyType_IsSubtype( derivedPyField -> pytype(), basePyField -> pytype() ) )
{
CSP_THROW( TypeError, "Field '" << derivedField -> fieldname() << "' on struct " << name
<< ": expected subclass of " << basePyField -> pytype() -> tp_name
<< ", got " << derivedPyField -> pytype() -> tp_name << " which is not a subclass" );
}
}
else if( baseTypeEnum == CspType::Type::STRUCT )
{
auto baseMeta = static_cast<const CspStructType &>( *baseField -> type() ).meta();
auto derivedMeta = static_cast<const CspStructType &>( *derivedField -> type() ).meta();
if( !StructMeta::isDerivedType( derivedMeta.get(), baseMeta.get() ) )
{
CSP_THROW( TypeError, "Field '" << derivedField -> fieldname() << "' on struct " << name
<< ": expected struct derived from " << baseMeta -> name()
<< ", got " << derivedMeta -> name() << " which is not a subclass" );
}
}
}
}

/*back reference to the struct type that will be accessible on the csp struct -> meta()
DialectStructMeta needs a strong reference to the type. This creates a known strong circular dep
Expand Down
Loading
Loading