[Core] Fix #15558: Direct expression in ternary operator (#22938)

* [Core] Fix #15558: Direct expression in ternary operator

* [Core] Added tests for Non-Numeric conditions in ternary op.

* [Core] prevent relational operator chains at grammar level.

* [Core] Rewrite expressions grammar as a layered grammar.

* [Core] Revert to left associative relops (like C/C++) plus tests.
This commit is contained in:
Frank David Martínez M
2025-08-25 14:52:42 -05:00
committed by GitHub
parent 0805164677
commit 0f2ea5588c
5 changed files with 565 additions and 464 deletions

View File

@@ -3264,7 +3264,7 @@ Expression *ConditionalExpression::simplify() const
if (!v)
return new ConditionalExpression(owner, condition->simplify(), trueExpr->simplify(), falseExpr->simplify());
else {
if (fabs(v->getValue()) > 0.5)
if (fabs(v->getValue()) >= Base::Precision::Confusion())
return trueExpr->simplify();
else
return falseExpr->simplify();

File diff suppressed because it is too large Load Diff

View File

@@ -45,7 +45,7 @@ std::stack<FunctionExpression::Function> functions; /**< Function
%token DOCUMENT OBJECT
%token EXPONENT
%type <arguments> args
%type <expr> input unit_num us_building_unit other_unit exp unit_exp cond indexable
%type <expr> input unit_num us_building_unit other_unit exp unit_exp indexable
%type <quantity> UNIT USUNIT
%type <string> id_or_cell STRING IDENTIFIER CELLADDRESS
%type <ivalue> INTEGER
@@ -61,8 +61,8 @@ std::stack<FunctionExpression::Function> functions; /**< Function
%type <string_or_identifier> document
%type <string_or_identifier> object
%type <ivalue> integer
%precedence EQ NEQ LT GT GTE LTE
%precedence ':'
%right '?' ':'
%left EQ NEQ LT GT GTE LTE
%left MINUSSIGN '+'
%left '*' '/' '%'
%precedence NUM_AND_UNIT
@@ -70,7 +70,7 @@ std::stack<FunctionExpression::Function> functions; /**< Function
%precedence NEG
%precedence POS
%destructor { delete $$; } num range exp cond unit_exp indexable
%destructor { delete $$; } num range exp unit_exp indexable
%destructor { delete $$; } <component>
%destructor { std::vector<Expression*>::const_iterator i = $$.begin(); while (i != $$.end()) { delete *i; ++i; } } args
@@ -99,9 +99,15 @@ exp: num { $$ = $1;
| exp '%' exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::MOD, $3); }
| exp '/' unit_exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::DIV, $3); }
| exp '^' exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::POW, $3); }
| exp EQ exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::EQ, $3); }
| exp NEQ exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::NEQ, $3); }
| exp LT exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::LT, $3); }
| exp GT exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::GT, $3); }
| exp GTE exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::GTE, $3); }
| exp LTE exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::LTE, $3); }
| indexable { $$ = $1; }
| FUNC args ')' { $$ = new FunctionExpression(DocumentObject, $1.first, std::move($1.second), $2);}
| cond '?' exp ':' exp { $$ = new ConditionalExpression(DocumentObject, $1, $3, $5); }
| exp '?' exp ':' exp { $$ = new ConditionalExpression(DocumentObject, $1, $3, $5); }
| '(' exp ')' { $$ = $2; }
;
@@ -112,26 +118,15 @@ num: ONE { $$ = new NumberExpression(Docu
args: exp { $$.push_back($1); }
| range { $$.push_back($1); }
| cond { $$.push_back($1); }
| args ',' exp { $1.push_back($3); $$ = $1; }
| args ';' exp { $1.push_back($3); $$ = $1; }
| args ',' range { $1.push_back($3); $$ = $1; }
| args ';' range { $1.push_back($3); $$ = $1; }
| args ',' cond { $1.push_back($3); $$ = $1; }
| args ';' cond { $1.push_back($3); $$ = $1; }
;
range: id_or_cell ':' id_or_cell { $$ = new RangeExpression(DocumentObject, $1, $3); }
;
cond: exp EQ exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::EQ, $3); }
| exp NEQ exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::NEQ, $3); }
| exp LT exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::LT, $3); }
| exp GT exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::GT, $3); }
| exp GTE exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::GTE, $3); }
| exp LTE exp { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::LTE, $3); }
| '(' cond ')' { $$ = $2; }
;
us_building_unit: USUNIT { $$ = new UnitExpression(DocumentObject, $1.scaler, $1.unitStr ); }
other_unit: UNIT { $$ = new UnitExpression(DocumentObject, $1.scaler, $1.unitStr ); }

View File

@@ -2,7 +2,7 @@
(cd "$(dirname "$0")" && \
flex -v -oExpression.lex.c Expression.l && \
bison -d -v -Wall -oExpression.tab.c Expression.y && \
bison -d -v -Wall -oExpression.tab.c -Wcounterexamples -Wconflicts-sr Expression.y && \
sed -i '1s|^|// clang-format off\n|' Expression.tab.c && \
sed -i '1s|^|// clang-format off\n|' Expression.lex.c \
)

View File

@@ -20,7 +20,6 @@
# ***************************************************************************/
import os
import sys
import math
from math import sqrt
import unittest
@@ -949,6 +948,48 @@ class SpreadsheetCases(unittest.TestCase):
self.assertEqual(sheet.getContents("A51"), "=+(-1 + 1)")
self.assertEqual(sheet.getContents("A52"), "=+(-1 + -1)")
# More ternary operator precedence tests
sheet.set("X1", "1")
sheet.set("X2", "10")
sheet.set("X3", "100")
sheet.set("X4", "1000")
sheet.set("Y1", "= X1 ? 0 : 1 ? X2 ? 2 : 3 : X3 ? 4 : 5")
sheet.set("Y2", "= X1 + X2 ? X3 - 2 : 3 + X4")
self.doc.recompute()
self.assertEqual(sheet.getContents("Y1"), "=X1 ? 0 : (1 ? (X2 ? 2 : 3) : (X3 ? 4 : 5))")
self.assertEqual(sheet.Y2, 98)
def testImplicitRelOpChains(self):
"""Test implicit chains of relational operators"""
sheet = self.doc.addObject("Spreadsheet::Sheet", "Spreadsheet")
def check(implicit: str, explicit: str):
sheet.set("Z1", implicit)
sheet.set("Z2", explicit)
self.doc.recompute()
self.assertEqual(sheet.getContents("Z2"), sheet.getContents("Z1"))
check("=1 < 2 < 3 ? 3 : 4", "=(1 < 2) < 3 ? 3 : 4")
check("=1 < 2 < 3 < 4 ? 3 : 4", "=((1 < 2) < 3) < 4 ? 3 : 4")
check("=A1 < B1 < 3 ? 3 : 4", "=(A1 < B1) < 3 ? 3 : 4")
check("=1 < C1 < D2 ? 3 : 4", "=(1 < C1) < D2 ? 3 : 4")
check("=X1 < B1 < D1 < 0 ? 3 : 4", "=((X1 < B1) < D1) < 0 ? 3 : 4")
def testExplicitRelOpChaining(self):
"""Testing explicit relational operators chaining."""
sheet = self.doc.addObject("Spreadsheet::Sheet", "Spreadsheet")
sheet.set("X1", "= (1 < 2) == (2 < 3)")
sheet.set("X2", "= (1 < 2) == (2 > 3)")
sheet.set("X3", "= (1 < 2) == 1")
sheet.set("X4", "= 0 == (1 > 2)")
self.doc.recompute()
self.assertTrue(sheet.get("X1"))
self.assertFalse(sheet.get("X2"))
self.assertTrue(sheet.get("X3"))
self.assertTrue(sheet.get("X4"))
def testNumbers(self):
"""Test different numbers"""
sheet = self.doc.addObject("Spreadsheet::Sheet", "Spreadsheet")
@@ -1879,3 +1920,99 @@ class SpreadsheetCases(unittest.TestCase):
self.assertLess(sheet.F3.distanceToPoint(FreeCAD.Vector(0.28, 0.04, -0.2)), tolerance)
self.assertLess(abs(sheet.F4.Value - -1.6971), 0.0001)
self.assertEqual(sheet.F5, FreeCAD.Vector(1.72, 2.96, 4.2))
def testTernaryOperator(self):
"""Testing ternary operator"""
sheet = self.doc.addObject("Spreadsheet::Sheet", "Spreadsheet")
sheet.set("A1", "= 1 ? 10 : 20")
sheet.set("A2", "= 0 ? 10 : 20")
sheet.set("A3", "= True ? 10 : 20")
sheet.set("A4", "= False ? 10 : 20")
sheet.set("B1", "= 1mm ? 10 : 20")
sheet.set("B2", "= 0mm ? 10 : 20")
sheet.set("C1", "= 2 == 2 ? 10 : 20")
sheet.set("C2", "= 2 != 2 ? 10 : 20")
sheet.set("C3", "= 2 > 1 ? 10 : 20")
sheet.set("C4", "= 1 > 2 ? 10 : 20")
sheet.set("D1", "= and(1 < 2, 3 < 4) ? 10 : 20")
sheet.set("D2", "= and(1 < 2, 3 > 4) ? 10 : 20")
sheet.set("D3", "= and(1 > 2, 3 < 4) ? 10 : 20")
sheet.set("D4", "= and(1 > 2, 3 > 4) ? 10 : 20")
sheet.set("E1", "= or(1 < 2, 3 < 4) ? 10 : 20")
sheet.set("E2", "= or(1 < 2, 3 > 4) ? 10 : 20")
sheet.set("E3", "= or(1 > 2, 3 < 4) ? 10 : 20")
sheet.set("E4", "= or(1 > 2, 3 > 4) ? 10 : 20")
sheet.set("F1", "= not(2 > 1) ? 10 : 20")
sheet.set("F2", "= not(2 < 1) ? 10 : 20")
sheet.addProperty("App::PropertyBool", "PropTrue", "", "")
sheet.PropTrue = True
sheet.addProperty("App::PropertyBool", "PropFalse", "", "")
sheet.PropFalse = False
sheet.addProperty("App::PropertyLength", "PropL", "", "")
sheet.PropL = "0 mm"
sheet.addProperty("App::PropertyLength", "PropH", "", "")
sheet.PropH = "5 mm"
sheet.addProperty("App::PropertyInteger", "PropI", "", "")
sheet.PropI = -5
sheet.addProperty("App::PropertyString", "PropS", "", "")
sheet.PropS = "This is a test"
sheet.addProperty("App::PropertyString", "PropSEmpty", "", "")
sheet.set("G1", "= Spreadsheet.PropTrue ? 10 : 20")
sheet.set("G2", "= Spreadsheet.PropFalse ? 10 : 20")
sheet.set("G3", "= Spreadsheet.PropL ? 10 : 20")
sheet.set("G4", "= Spreadsheet.PropH ? 10 : 20")
sheet.set("G5", "= Spreadsheet.PropI ? 10 : 20")
sheet.set("G6", "= Spreadsheet.PropS ? 10 : 20")
sheet.set("G7", "= Spreadsheet.PropSEmpty ? 10 : 20")
sheet.set("G8", "= <<Test>> ? 10 : 20")
sheet.set("G9", "= <<>> ? 10 : 20")
sheet.set("G10", "= <<Spreadsheet>> ? 10 : 20")
self.doc.recompute()
# Constants
self.assertEqual(sheet.get("A1"), 10)
self.assertEqual(sheet.get("A2"), 20)
self.assertEqual(sheet.get("A3"), 10)
self.assertEqual(sheet.get("A4"), 20)
# Constant Quantities
self.assertEqual(sheet.get("B1"), 10)
self.assertEqual(sheet.get("B2"), 20)
# Basic Comparisons
self.assertEqual(sheet.get("C1"), 10)
self.assertEqual(sheet.get("C2"), 20)
self.assertEqual(sheet.get("C3"), 10)
self.assertEqual(sheet.get("C4"), 20)
# Function calls
self.assertEqual(sheet.get("D1"), 10)
self.assertEqual(sheet.get("D2"), 20)
self.assertEqual(sheet.get("D3"), 20)
self.assertEqual(sheet.get("D4"), 20)
self.assertEqual(sheet.get("E1"), 10)
self.assertEqual(sheet.get("E2"), 10)
self.assertEqual(sheet.get("E3"), 10)
self.assertEqual(sheet.get("E4"), 20)
self.assertEqual(sheet.get("F1"), 20)
self.assertEqual(sheet.get("F2"), 10)
# Property lookup
self.assertEqual(sheet.get("G1"), 10)
self.assertEqual(sheet.get("G2"), 20)
self.assertEqual(sheet.get("G3"), 20)
self.assertEqual(sheet.get("G4"), 10)
self.assertEqual(sheet.get("G5"), 10)
# Non numeric conditions
self.assertEqual(sheet.get("G6"), 10)
self.assertEqual(sheet.get("G7"), 20)
self.assertEqual(sheet.get("G8"), 10)
self.assertEqual(sheet.get("G9"), 20)
self.assertEqual(sheet.get("G10"), 10)