Add rudimentary type checking to rule eval with reporter.

Also cleaned up existing error reporting to not expend CPU cycles
when there was no reporter.

Fixes https://gitlab.com/kicad/code/kicad/issues/8438
This commit is contained in:
Jeff Young 2021-05-20 23:05:27 +01:00
parent e8102d85dc
commit e93e9fa3e5
5 changed files with 79 additions and 38 deletions

View File

@ -656,13 +656,8 @@ void dumpNode( wxString& buf, TREE_NODE* tok, int depth = 0 )
void CONTEXT::ReportError( const wxString& aErrorMsg )
{
m_errorStatus.pendingError = true;
m_errorStatus.message = aErrorMsg;
m_errorStatus.srcPos = -1;
m_errorStatus.stage = CST_RUNTIME;
if( m_errorCallback )
m_errorCallback( m_errorStatus.message, m_errorStatus.srcPos );
m_errorCallback( aErrorMsg, -1 );
}
@ -1117,6 +1112,22 @@ void UOP::Exec( CONTEXT* ctx )
double arg1Value = arg1 ? arg1->AsDouble() : 0.0;
double result;
if( ctx->HasErrorCallback() )
{
if( arg1->GetType() == VT_STRING && arg2->GetType() == VT_NUMERIC )
{
ctx->ReportError( wxString::Format( _( "Type mismatch between '%s' and %lf" ),
arg1->AsString(),
arg2->AsDouble() ) );
}
else if( arg1->GetType() == VT_NUMERIC && arg2->GetType() == VT_STRING )
{
ctx->ReportError( wxString::Format( _( "Type mismatch between %lf and '%s'" ),
arg1->AsDouble(),
arg2->AsString() ) );
}
}
switch( m_op )
{
case TR_OP_ADD:

View File

@ -311,15 +311,14 @@ public:
m_errorCallback = std::move( aCallback );
}
bool HasErrorCallback() { return m_errorCallback != nullptr; }
void ReportError( const wxString& aErrorMsg );
bool IsErrorPending() const { return m_errorStatus.pendingError; }
const ERROR_STATUS& GetError() const { return m_errorStatus; }
private:
std::vector<VALUE*> m_ownedValues;
VALUE* m_stack[100]; // std::stack not performant enough
int m_stackPtr;
ERROR_STATUS m_errorStatus;
std::function<void( const wxString& aMessage, int aOffset )> m_errorCallback;
};

View File

@ -55,12 +55,15 @@ bool DRC_RULE_CONDITION::EvaluateFor( const BOARD_ITEM* aItemA, const BOARD_ITEM
}
PCB_EXPR_CONTEXT ctx( aLayer );
ctx.SetErrorCallback(
[&]( const wxString& aMessage, int aOffset )
{
if( aReporter )
if( aReporter )
{
ctx.SetErrorCallback(
[&]( const wxString& aMessage, int aOffset )
{
aReporter->Report( _( "ERROR:" ) + wxS( " " )+ aMessage );
} );
} );
}
BOARD_ITEM* a = const_cast<BOARD_ITEM*>( aItemA );
BOARD_ITEM* b = const_cast<BOARD_ITEM*>( aItemB );
@ -87,10 +90,10 @@ bool DRC_RULE_CONDITION::Compile( REPORTER* aReporter, int aSourceLine, int aSou
{
PCB_EXPR_COMPILER compiler;
compiler.SetErrorCallback(
[&]( const wxString& aMessage, int aOffset )
{
if( aReporter )
if( aReporter )
{
compiler.SetErrorCallback(
[&]( const wxString& aMessage, int aOffset )
{
wxString rest;
wxString first = aMessage.BeforeFirst( '|', &rest );
@ -101,8 +104,8 @@ bool DRC_RULE_CONDITION::Compile( REPORTER* aReporter, int aSourceLine, int aSou
rest );
aReporter->Report( msg, RPT_SEVERITY_ERROR );
}
} );
} );
}
m_ucode = std::make_unique<PCB_EXPR_UCODE>();

View File

@ -134,7 +134,7 @@ void DRC_RULES_PARSER::Parse( std::vector<DRC_RULE*>& aRules, REPORTER* aReporte
}
else
{
msg.Printf( _( "Unrecognized item '%s'.| Expected version number" ),
msg.Printf( _( "Unrecognized item '%s'.| Expected version number." ),
FromUTF8() );
reportError( msg );
}

View File

@ -83,7 +83,7 @@ static void existsOnLayer( LIBEVAL::CONTEXT* aCtx, void *self )
if( !item )
return;
if( !arg )
if( !arg && aCtx->HasErrorCallback() )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "existsOnLayer()" ) ) );
@ -110,7 +110,7 @@ static void existsOnLayer( LIBEVAL::CONTEXT* aCtx, void *self )
}
}
if( !anyMatch )
if( !anyMatch && aCtx->HasErrorCallback() )
aCtx->ReportError( wxString::Format( _( "Unrecognized layer '%s'" ), layerName ) );
}
@ -180,8 +180,12 @@ static void insideCourtyard( LIBEVAL::CONTEXT* aCtx, void* self )
if( !arg )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "insideCourtyard()" ) ) );
if( aCtx->HasErrorCallback() )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "insideCourtyard()" ) ) );
}
return;
}
@ -257,8 +261,12 @@ static void insideFrontCourtyard( LIBEVAL::CONTEXT* aCtx, void* self )
if( !arg )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "insideFrontCourtyard()" ) ) );
if( aCtx->HasErrorCallback() )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "insideFrontCourtyard()" ) ) );
}
return;
}
@ -335,8 +343,11 @@ static void insideBackCourtyard( LIBEVAL::CONTEXT* aCtx, void* self )
if( !arg )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "insideBackCourtyard()" ) ) );
if( aCtx->HasErrorCallback() )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "insideBackCourtyard()" ) ) );
}
return;
}
@ -413,8 +424,12 @@ static void insideArea( LIBEVAL::CONTEXT* aCtx, void* self )
if( !arg )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "insideArea()" ) ) );
if( aCtx->HasErrorCallback() )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "insideArea()" ) ) );
}
return;
}
@ -471,7 +486,9 @@ static void insideArea( LIBEVAL::CONTEXT* aCtx, void* self )
if( ( footprint->GetFlags() & MALFORMED_COURTYARDS ) != 0 )
{
aCtx->ReportError( _( "Footprint's courtyard is not a single, closed shape." ) );
if( aCtx->HasErrorCallback() )
aCtx->ReportError( _( "Footprint's courtyard is not a single, closed shape." ) );
return false;
}
@ -481,7 +498,9 @@ static void insideArea( LIBEVAL::CONTEXT* aCtx, void* self )
if( courtyard.OutlineCount() == 0 )
{
aCtx->ReportError( _( "Footprint has no front courtyard." ) );
if( aCtx->HasErrorCallback() )
aCtx->ReportError( _( "Footprint has no front courtyard." ) );
return false;
}
else
@ -496,7 +515,9 @@ static void insideArea( LIBEVAL::CONTEXT* aCtx, void* self )
if( courtyard.OutlineCount() == 0 )
{
aCtx->ReportError( _( "Footprint has no back courtyard." ) );
if( aCtx->HasErrorCallback() )
aCtx->ReportError( _( "Footprint has no back courtyard." ) );
return false;
}
else
@ -643,8 +664,11 @@ static void memberOf( LIBEVAL::CONTEXT* aCtx, void* self )
if( !arg )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "memberOf()" ) ) );
if( aCtx->HasErrorCallback() )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "memberOf()" ) ) );
}
return;
}
@ -744,8 +768,12 @@ static void inDiffPair( LIBEVAL::CONTEXT* aCtx, void* self )
if( !arg )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "inDiffPair()" ) ) );
if( aCtx->HasErrorCallback() )
{
aCtx->ReportError( wxString::Format( _( "Missing argument to '%s'" ),
wxT( "inDiffPair()" ) ) );
}
return;
}