Better and more complete recursion guards.

(ie: ones that actually work.)

Fixes https://gitlab.com/kicad/code/kicad/-/issues/20990
This commit is contained in:
Jeff Young 2025-05-24 22:04:49 +01:00
parent 1c8f971789
commit ed1b4f6e5a
7 changed files with 20 additions and 30 deletions

View File

@ -182,7 +182,7 @@ bool DIALOG_SIM_MODEL<T>::TransferDataToWindow()
WX_STRING_REPORTER reporter; WX_STRING_REPORTER reporter;
// Infer RLC and VI models if they aren't specified // Infer RLC and VI models if they aren't specified
if( SIM_MODEL::InferSimModel( m_symbol, &m_fields, false, SIM_VALUE_GRAMMAR::NOTATION::SI, if( SIM_MODEL::InferSimModel( m_symbol, &m_fields, false, 0, SIM_VALUE_GRAMMAR::NOTATION::SI,
&deviceType, &modelType, &modelParams, &pinMap ) ) &deviceType, &modelType, &modelParams, &pinMap ) )
{ {
SetFieldValue( m_fields, SIM_DEVICE_FIELD, deviceType.ToStdString() ); SetFieldValue( m_fields, SIM_DEVICE_FIELD, deviceType.ToStdString() );

View File

@ -254,18 +254,8 @@ wxString SCH_FIELD::GetShownText( const SCH_SHEET_PATH* aPath, bool aAllowExtraT
if( HasTextVars() ) if( HasTextVars() )
{ {
wxString last; while( text.Contains( wxT( "${" ) ) && aDepth++ <= ADVANCED_CFG::GetCfg().m_ResolveTextRecursionDepth )
int iterration = aDepth;
// The iteration here it to allow for nested variables in the
// text strings (e.g. ${${VAR}}). Although the symbols and sheets
// and labels recurse, text that is none of those types such as text
// boxes and labels do not. This only loops if there is still a
// variable to resolve.
do
{ {
last = text;
if( m_parent && m_parent->Type() == LIB_SYMBOL_T ) if( m_parent && m_parent->Type() == LIB_SYMBOL_T )
text = ExpandTextVars( text, &libSymbolResolver ); text = ExpandTextVars( text, &libSymbolResolver );
else if( m_parent && m_parent->Type() == SCH_SYMBOL_T ) else if( m_parent && m_parent->Type() == SCH_SYMBOL_T )
@ -279,7 +269,7 @@ wxString SCH_FIELD::GetShownText( const SCH_SHEET_PATH* aPath, bool aAllowExtraT
text = ExpandTextVars( text, &Schematic()->Prj() ); text = ExpandTextVars( text, &Schematic()->Prj() );
text = ExpandTextVars( text, &schematicResolver ); text = ExpandTextVars( text, &schematicResolver );
} }
} while( text != last && iterration++ <= ADVANCED_CFG::GetCfg().m_ResolveTextRecursionDepth ); }
} }
if( m_id == FIELD_T::REFERENCE && aPath ) if( m_id == FIELD_T::REFERENCE && aPath )

View File

@ -407,13 +407,13 @@ inline wxString GetFieldValue( const std::vector<SCH_FIELD>* aFields, FIELD_T aF
inline std::string GetFieldValue( const std::vector<SCH_FIELD>* aFields, inline std::string GetFieldValue( const std::vector<SCH_FIELD>* aFields,
const wxString& aFieldName, bool aResolve = false ) const wxString& aFieldName, bool aResolve = false, int aDepth = 0 )
{ {
if( !aFields ) if( !aFields )
return ""; return "";
if( const SCH_FIELD* field = FindField( *aFields, aFieldName ) ) if( const SCH_FIELD* field = FindField( *aFields, aFieldName ) )
return ( aResolve ? field->GetShownText( false ) : field->GetText() ).ToStdString(); return ( aResolve ? field->GetShownText( false, aDepth ) : field->GetText() ).ToStdString();
return ""; return "";
} }

View File

@ -227,7 +227,7 @@ SIM_LIBRARY::MODEL SIM_LIB_MGR::CreateModel( const SCH_SHEET_PATH* aSheetPath, S
bool storeInValue = false; bool storeInValue = false;
// Infer RLC and VI models if they aren't specified // Infer RLC and VI models if they aren't specified
if( SIM_MODEL::InferSimModel( aSymbol, &fields, true, SIM_VALUE_GRAMMAR::NOTATION::SI, if( SIM_MODEL::InferSimModel( aSymbol, &fields, true, aDepth, SIM_VALUE_GRAMMAR::NOTATION::SI,
&deviceType, &modelType, &modelParams, &pinMap ) ) &deviceType, &modelType, &modelParams, &pinMap ) )
{ {
getOrCreateField( SIM_DEVICE_FIELD )->SetText( deviceType ); getOrCreateField( SIM_DEVICE_FIELD )->SetText( deviceType );

View File

@ -1016,7 +1016,7 @@ bool SIM_MODEL::requiresSpiceModelLine( const SPICE_ITEM& aItem ) const
template <class T> template <class T>
bool SIM_MODEL::InferSimModel( T& aSymbol, std::vector<SCH_FIELD>* aFields, bool aResolve, bool SIM_MODEL::InferSimModel( T& aSymbol, std::vector<SCH_FIELD>* aFields, bool aResolve, int aDepth,
SIM_VALUE_GRAMMAR::NOTATION aNotation, wxString* aDeviceType, SIM_VALUE_GRAMMAR::NOTATION aNotation, wxString* aDeviceType,
wxString* aModelType, wxString* aModelParams, wxString* aPinMap ) wxString* aModelType, wxString* aModelParams, wxString* aPinMap )
{ {
@ -1178,9 +1178,9 @@ bool SIM_MODEL::InferSimModel( T& aSymbol, std::vector<SCH_FIELD>* aFields, bool
}; };
wxString prefix = aSymbol.GetPrefix(); wxString prefix = aSymbol.GetPrefix();
wxString library = GetFieldValue( aFields, SIM_LIBRARY_FIELD, aResolve ); wxString library = GetFieldValue( aFields, SIM_LIBRARY_FIELD, aResolve, aDepth );
wxString modelName = GetFieldValue( aFields, SIM_NAME_FIELD, aResolve ); wxString modelName = GetFieldValue( aFields, SIM_NAME_FIELD, aResolve, aDepth );
wxString value = GetFieldValue( aFields, SIM_VALUE_FIELD, aResolve ); wxString value = GetFieldValue( aFields, SIM_VALUE_FIELD, aResolve, aDepth );
std::vector<SCH_PIN*> pins = aSymbol.GetPins(); std::vector<SCH_PIN*> pins = aSymbol.GetPins();
// ensure the pins are sorted by number (not guaranteed in the symbol) // ensure the pins are sorted by number (not guaranteed in the symbol)
@ -1192,10 +1192,10 @@ bool SIM_MODEL::InferSimModel( T& aSymbol, std::vector<SCH_FIELD>* aFields, bool
return a->GetNumber() < b->GetNumber(); return a->GetNumber() < b->GetNumber();
} ); } );
*aDeviceType = GetFieldValue( aFields, SIM_DEVICE_FIELD, aResolve ); *aDeviceType = GetFieldValue( aFields, SIM_DEVICE_FIELD, aResolve, aDepth );
*aModelType = GetFieldValue( aFields, SIM_DEVICE_SUBTYPE_FIELD, aResolve ); *aModelType = GetFieldValue( aFields, SIM_DEVICE_SUBTYPE_FIELD, aResolve, aDepth );
*aModelParams = GetFieldValue( aFields, SIM_PARAMS_FIELD, aResolve ); *aModelParams = GetFieldValue( aFields, SIM_PARAMS_FIELD, aResolve, aDepth );
*aPinMap = GetFieldValue( aFields, SIM_PINS_FIELD, aResolve ); *aPinMap = GetFieldValue( aFields, SIM_PINS_FIELD, aResolve, aDepth );
if( pins.size() != 2 ) if( pins.size() != 2 )
return false; return false;
@ -1342,13 +1342,13 @@ bool SIM_MODEL::InferSimModel( T& aSymbol, std::vector<SCH_FIELD>* aFields, bool
} }
template bool SIM_MODEL::InferSimModel<SCH_SYMBOL>( SCH_SYMBOL& aSymbol, template bool SIM_MODEL::InferSimModel<SCH_SYMBOL>( SCH_SYMBOL& aSymbol, std::vector<SCH_FIELD>* aFields,
std::vector<SCH_FIELD>* aFields, bool aResolve, bool aResolve, int aDepth,
SIM_VALUE_GRAMMAR::NOTATION aNotation, SIM_VALUE_GRAMMAR::NOTATION aNotation,
wxString* aDeviceType, wxString* aModelType, wxString* aDeviceType, wxString* aModelType,
wxString* aModelParams, wxString* aPinMap ); wxString* aModelParams, wxString* aPinMap );
template bool SIM_MODEL::InferSimModel<LIB_SYMBOL>( LIB_SYMBOL& aSymbol, template bool SIM_MODEL::InferSimModel<LIB_SYMBOL>( LIB_SYMBOL& aSymbol, std::vector<SCH_FIELD>* aFields,
std::vector<SCH_FIELD>* aFields, bool aResolve, bool aResolve, int aDepth,
SIM_VALUE_GRAMMAR::NOTATION aNotation, SIM_VALUE_GRAMMAR::NOTATION aNotation,
wxString* aDeviceType, wxString* aModelType, wxString* aDeviceType, wxString* aModelType,
wxString* aModelParams, wxString* aPinMap ); wxString* aModelParams, wxString* aPinMap );

View File

@ -504,7 +504,7 @@ public:
virtual void SwitchSingleEndedDiff( bool aDiff ) { }; virtual void SwitchSingleEndedDiff( bool aDiff ) { };
template <class T> template <class T>
static bool InferSimModel( T& aSymbol, std::vector<SCH_FIELD>* aFields, bool aResolve, static bool InferSimModel( T& aSymbol, std::vector<SCH_FIELD>* aFields, bool aResolve, int aDepth,
SIM_VALUE_GRAMMAR::NOTATION aNotation, wxString* aDeviceType, SIM_VALUE_GRAMMAR::NOTATION aNotation, wxString* aDeviceType,
wxString* aModelType, wxString* aModelParams, wxString* aPinMap ); wxString* aModelType, wxString* aModelParams, wxString* aPinMap );

View File

@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE( InferPassiveValues )
fields.emplace_back( symbol->GetReferenceField() ); fields.emplace_back( symbol->GetReferenceField() );
fields.emplace_back( symbol->GetValueField() ); fields.emplace_back( symbol->GetValueField() );
SIM_MODEL::InferSimModel( *symbol, &fields, false, SIM_VALUE_GRAMMAR::NOTATION::SPICE, SIM_MODEL::InferSimModel( *symbol, &fields, false, 0, SIM_VALUE_GRAMMAR::NOTATION::SPICE,
&deviceType, &modelType, &modelParams, &pinMap ); &deviceType, &modelType, &modelParams, &pinMap );
msg.Printf( "Passive model inference %s %s failed [%s != %s]", msg.Printf( "Passive model inference %s %s failed [%s != %s]",