Bug fixes for layer expression processing.

1) Push a VAR onto the stack, not a resolved value
2) Don't collapse a PCB_LAYER_VALUE to a VALUE during processing
3) Make sure we run overloaded operators from the correct side

Fixes https://gitlab.com/kicad/code/kicad/issues/12437

common/libeval_compiler/libeval_compiler.cpp

@@ -1038,7 +1038,7 @@ bool COMPILER::generateUCode( UCODE* aCode, CONTEXT* aPreflightContext )
                 reportError( CST_CODEGEN, msg, node->srcPos - (int) node->value.str->length() );
             }
 
-            node->SetUop( TR_UOP_PUSH_VALUE, std::move( vref ) );
+            node->SetUop( TR_UOP_PUSH_VAR, std::move( vref ) );
             node->isTerminal = true;
             break;
         }
@@ -1090,10 +1090,12 @@ void UOP::Exec( CONTEXT* ctx )
     {
     case TR_UOP_PUSH_VAR:
     {
-        VALUE* value = ctx->AllocValue();
+        VALUE* value = nullptr;
 
         if( m_ref )
-            value->Set( m_ref->GetValue( ctx ) );
+            value = ctx->StoreValue( m_ref->GetValue( ctx ) );
+        else
+            value = ctx->AllocValue();
 
         ctx->Push( value );
     }
@@ -1162,10 +1164,20 @@ void UOP::Exec( CONTEXT* ctx )
             result = arg1Value > arg2Value ? 1 : 0;
             break;
         case TR_OP_EQUAL:
-            result = arg1 && arg2 && arg1->EqualTo( ctx, arg2 ) ? 1 : 0;
+            if( !arg1 || !arg2 )
+                result = arg1 == arg2 ? 1 : 0;
+            else if( arg2->GetType() == VT_UNDEFINED )
+                result = arg2->EqualTo( ctx, arg1 ) ? 1 : 0;
+            else
+                result = arg1->EqualTo( ctx, arg2 ) ? 1 : 0;
             break;
         case TR_OP_NOT_EQUAL:
-            result = arg1 && arg2 && arg1->NotEqualTo( ctx, arg2 ) ? 1 : 0;
+            if( !arg1 || !arg2 )
+                result = arg1 != arg2 ? 1 : 0;
+            else if( arg2->GetType() == VT_UNDEFINED )
+                result = arg2->NotEqualTo( ctx, arg1 ) ? 1 : 0;
+            else
+                result = arg1->NotEqualTo( ctx, arg2 ) ? 1 : 0;
             break;
         case TR_OP_BOOL_AND:
             result = arg1Value != 0.0 && arg2Value != 0.0 ? 1 : 0;

include/libeval_compiler/libeval_compiler.h

@@ -296,7 +296,7 @@ public:
     virtual ~VAR_REF() {};
 
     virtual VAR_TYPE_T GetType() const = 0;
-    virtual VALUE GetValue( CONTEXT* aCtx ) = 0;
+    virtual VALUE* GetValue( CONTEXT* aCtx ) = 0;
 };
 
 
@@ -312,7 +312,7 @@ public:
 
     virtual ~CONTEXT()
     {
-        for( auto &v : m_ownedValues )
+        for( VALUE* v : m_ownedValues )
         {
             delete v;
         }
@@ -324,6 +324,12 @@ public:
         return m_ownedValues.back();
     }
 
+    VALUE* StoreValue( VALUE* aValue )
+    {
+        m_ownedValues.emplace_back( aValue );
+        return m_ownedValues.back();
+    }
+
     void Push( VALUE* v )
     {
         m_stack[ m_stackPtr++ ] = v;

pcbnew/dialogs/panel_setup_rules_help.md

@@ -48,6 +48,14 @@
  * zone\_connection
 
 Note: `clearance` and `hole_clearance` rules are not run against items of the same net; `physical_clearance` and `physical_hole_clearance` rules are.
+<br><br>
+
+### Items
+
+ * `A` &nbsp;&nbsp; _the first (or only) item under test_
+ * `B` &nbsp;&nbsp; _the second item under test (for binary tests)_
+ * `L` &nbsp;&nbsp; _the layer currently under test_
+
 <br>
 
 ### Item Types

pcbnew/dialogs/panel_setup_rules_help_md.h

@@ -49,6 +49,14 @@ _HKI( "### Top-level Clauses\n"
 " * zone\\_connection\n"
 "\n"
 "Note: `clearance` and `hole_clearance` rules are not run against items of the same net; `physical_clearance` and `physical_hole_clearance` rules are.\n"
+"<br><br>\n"
+"\n"
+"### Items\n"
+"\n"
+" * `A` &nbsp;&nbsp; _the first (or only) item under test_\n"
+" * `B` &nbsp;&nbsp; _the second item under test (for binary tests)_\n"
+" * `L` &nbsp;&nbsp; _the layer currently under test_\n"
+"\n"
 "<br>\n"
 "\n"
 "### Item Types\n"

pcbnew/pcb_expr_evaluator.cpp

@@ -992,7 +992,8 @@ class PCB_LAYER_VALUE : public LIBEVAL::VALUE
 {
 public:
     PCB_LAYER_VALUE( PCB_LAYER_ID aLayer ) :
-        LIBEVAL::VALUE( double( aLayer ) )
+        LIBEVAL::VALUE( LayerName( aLayer ) ),
+        m_layer( aLayer )
     {};
 
     virtual bool EqualTo( LIBEVAL::CONTEXT* aCtx, const VALUE* b ) const override
@@ -1025,25 +1026,25 @@ public:
             mask = i->second;
         }
 
-        PCB_LAYER_ID layerId = ToLAYER_ID( (int) AsDouble() );
-
-        return mask.Contains( layerId );
+        return mask.Contains( m_layer );
     }
+
+protected:
+    PCB_LAYER_ID m_layer;
 };
 
 
-LIBEVAL::VALUE PCB_EXPR_VAR_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
+LIBEVAL::VALUE* PCB_EXPR_VAR_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
 {
+    PCB_EXPR_CONTEXT* context = static_cast<PCB_EXPR_CONTEXT*>( aCtx );
+
     if( m_itemIndex == 2 )
-    {
-        PCB_EXPR_CONTEXT* context = static_cast<PCB_EXPR_CONTEXT*>( aCtx );
-        return PCB_LAYER_VALUE( context->GetLayer() );
-    }
+        return new PCB_LAYER_VALUE( context->GetLayer() );
 
     BOARD_ITEM* item = GetObject( aCtx );
 
     if( !item )
-        return LIBEVAL::VALUE();
+        return new LIBEVAL::VALUE();
 
     auto it = m_matchingTypes.find( TYPE_HASH( *item ) );
 
@@ -1053,12 +1054,12 @@ LIBEVAL::VALUE PCB_EXPR_VAR_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
         // simpler "A.Via_Type == 'buried'" is perfectly clear.  Instead, return an undefined
         // value when the property doesn't appear on a particular object.
 
-        return LIBEVAL::VALUE();
+        return new LIBEVAL::VALUE();
     }
     else
     {
         if( m_type == LIBEVAL::VT_NUMERIC )
-            return LIBEVAL::VALUE( (double) item->Get<int>( it->second ) );
+            return new LIBEVAL::VALUE( (double) item->Get<int>( it->second ) );
         else
         {
             wxString str;
@@ -1066,7 +1067,7 @@ LIBEVAL::VALUE PCB_EXPR_VAR_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
             if( !m_isEnum )
             {
                 str = item->Get<wxString>( it->second );
-                return LIBEVAL::VALUE( str );
+                return new LIBEVAL::VALUE( str );
             }
             else
             {
@@ -1074,45 +1075,50 @@ LIBEVAL::VALUE PCB_EXPR_VAR_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
                 bool valid = any.GetAs<wxString>( &str );
 
                 if( valid )
-                    return LIBEVAL::VALUE( str );
+                {
+                    if( it->second->Name() == wxT( "Layer" ) )
+                        return new PCB_LAYER_VALUE( context->GetBoard()->GetLayerID( str ) );
+                    else
+                        return new LIBEVAL::VALUE( str );
+                }
             }
 
-            return LIBEVAL::VALUE();
+            return new LIBEVAL::VALUE();
         }
     }
 }
 
 
-LIBEVAL::VALUE PCB_EXPR_NETCLASS_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
+LIBEVAL::VALUE* PCB_EXPR_NETCLASS_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
 {
     BOARD_CONNECTED_ITEM* item = dynamic_cast<BOARD_CONNECTED_ITEM*>( GetObject( aCtx ) );
 
     if( !item )
-        return LIBEVAL::VALUE();
+        return new LIBEVAL::VALUE();
 
-    return LIBEVAL::VALUE( item->GetEffectiveNetClass()->GetName() );
+    return new LIBEVAL::VALUE( item->GetEffectiveNetClass()->GetName() );
 }
 
 
-LIBEVAL::VALUE PCB_EXPR_NETNAME_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
+LIBEVAL::VALUE* PCB_EXPR_NETNAME_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
 {
     BOARD_CONNECTED_ITEM* item = dynamic_cast<BOARD_CONNECTED_ITEM*>( GetObject( aCtx ) );
 
     if( !item )
-        return LIBEVAL::VALUE();
+        return new LIBEVAL::VALUE();
 
-    return LIBEVAL::VALUE( item->GetNetname() );
+    return new LIBEVAL::VALUE( item->GetNetname() );
 }
 
 
-LIBEVAL::VALUE PCB_EXPR_TYPE_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
+LIBEVAL::VALUE* PCB_EXPR_TYPE_REF::GetValue( LIBEVAL::CONTEXT* aCtx )
 {
     BOARD_ITEM* item = GetObject( aCtx );
 
     if( !item )
-        return LIBEVAL::VALUE();
+        return new LIBEVAL::VALUE();
 
-    return LIBEVAL::VALUE( ENUM_MAP<KICAD_T>::Instance().ToString( item->Type() ) );
+    return new LIBEVAL::VALUE( ENUM_MAP<KICAD_T>::Instance().ToString( item->Type() ) );
 }
 
 

pcbnew/pcb_expr_evaluator.h

@@ -103,7 +103,7 @@ public:
         m_matchingTypes[type_hash] = prop;
     }
 
-    virtual LIBEVAL::VALUE GetValue( LIBEVAL::CONTEXT* aCtx ) override;
+    LIBEVAL::VALUE* GetValue( LIBEVAL::CONTEXT* aCtx ) override;
 
     BOARD_ITEM* GetObject( const LIBEVAL::CONTEXT* aCtx ) const;
 
@@ -126,7 +126,7 @@ public:
         //printf("*** CreateVarRef %p %d\n", this, aItemIndex );
     }
 
-    LIBEVAL::VALUE GetValue( LIBEVAL::CONTEXT* aCtx ) override;
+    LIBEVAL::VALUE* GetValue( LIBEVAL::CONTEXT* aCtx ) override;
 };
 
 
@@ -141,7 +141,7 @@ public:
         //printf("*** CreateVarRef %p %d\n", this, aItemIndex );
     }
 
-    LIBEVAL::VALUE GetValue( LIBEVAL::CONTEXT* aCtx ) override;
+    LIBEVAL::VALUE* GetValue( LIBEVAL::CONTEXT* aCtx ) override;
 };
 
 
@@ -155,7 +155,7 @@ public:
         //printf("*** CreateVarRef %p %d\n", this, aItemIndex );
     }
 
-    LIBEVAL::VALUE GetValue( LIBEVAL::CONTEXT* aCtx ) override;
+    LIBEVAL::VALUE* GetValue( LIBEVAL::CONTEXT* aCtx ) override;
 };