ExpressionParser: Return unique_ptr from simplified (#27098)

- Removes many/most of the copy().release() from the last commit
- Still some .release() in the simplify functions now
- It doesn't work to rename simplify and turn it into a protected
  method, because it's called recursively within the tree
This commit is contained in:
timpieces
2026-01-23 12:34:25 +08:00
parent 8dad760db4
commit daceb8afe4
4 changed files with 63 additions and 57 deletions
+49 -43
View File
@@ -1226,9 +1226,9 @@ void UnitExpression::setUnit(const Quantity &_quantity)
}
}
Expression *UnitExpression::simplify() const
ExpressionPtr UnitExpression::simplify() const
{
return new NumberExpression(owner, quantity);
return std::make_unique<NumberExpression>(owner, quantity);
}
void UnitExpression::_toString(std::ostream &ss, bool,int) const
@@ -1258,9 +1258,9 @@ NumberExpression::NumberExpression(const DocumentObject *_owner, const Quantity
{
}
Expression *NumberExpression::simplify() const
ExpressionPtr NumberExpression::simplify() const
{
return copy().release();
return copy();
}
Expression *NumberExpression::_copy() const
@@ -1439,19 +1439,17 @@ Py::Object OperatorExpression::_getPyValue() const {
return calc(this,op,left,right,false);
}
Expression *OperatorExpression::simplify() const
ExpressionPtr OperatorExpression::simplify() const
{
Expression * v1 = left->simplify();
Expression * v2 = right->simplify();
ExpressionPtr v1 = left->simplify();
ExpressionPtr v2 = right->simplify();
// Both arguments reduced to numerics? Then evaluate and return answer
if (freecad_cast<NumberExpression*>(v1) && freecad_cast<NumberExpression*>(v2)) {
delete v1;
delete v2;
return eval().release();
if (freecad_cast<NumberExpression*>(v1.get()) && freecad_cast<NumberExpression*>(v2.get())) {
return eval();
}
else
return new OperatorExpression(owner, v1, op, v2);
return std::make_unique<OperatorExpression>(owner, v1.release(), op, v2.release());
}
void OperatorExpression::_toString(std::ostream &s, bool persistent,int) const
@@ -2587,18 +2585,19 @@ Py::Object FunctionExpression::_getPyValue() const {
return evaluate(this,f,args);
}
Expression *FunctionExpression::simplify() const
ExpressionPtr FunctionExpression::simplify() const
{
size_t numerics = 0;
std::vector<Expression*> simplifiedArgs;
// Try to simplify each argument to function
for (auto it : args) {
Expression * v = it->simplify();
ExpressionPtr v = it->simplify();
if (freecad_cast<NumberExpression*>(v))
if (freecad_cast<NumberExpression*>(v.get())) {
++numerics;
simplifiedArgs.push_back(v);
}
simplifiedArgs.push_back(v.release());
}
if (numerics == args.size()) {
@@ -2608,11 +2607,15 @@ Expression *FunctionExpression::simplify() const
for (auto it : simplifiedArgs)
delete it;
return eval().release();
return eval();
}
else
return new FunctionExpression(owner, f, std::string(fname),
std::move(simplifiedArgs));
return std::make_unique<FunctionExpression>(
owner,
f,
std::string(fname),
std::move(simplifiedArgs)
);
}
void FunctionExpression::_toString(std::ostream &ss, bool persistent,int) const
@@ -2878,9 +2881,9 @@ void VariableExpression::_toString(std::ostream &ss, bool persistent,int) const
ss << var.toString();
}
Expression *VariableExpression::simplify() const
ExpressionPtr VariableExpression::simplify() const
{
return copy().release();
return copy();
}
Expression *VariableExpression::_copy() const
@@ -3083,9 +3086,9 @@ StringExpression::~StringExpression() {
}
}
Expression *StringExpression::simplify() const
ExpressionPtr StringExpression::simplify() const
{
return copy().release();
return copy();
}
void StringExpression::_toString(std::ostream &ss, bool,int) const
@@ -3131,18 +3134,26 @@ Py::Object ConditionalExpression::_getPyValue() const {
return falseExpr->getPyValue();
}
Expression *ConditionalExpression::simplify() const
ExpressionPtr ConditionalExpression::simplify() const
{
std::unique_ptr<Expression> e(condition->simplify());
ExpressionPtr e = condition->simplify();
NumberExpression * v = freecad_cast<NumberExpression*>(e.get());
if (!v)
return new ConditionalExpression(owner, condition->simplify(), trueExpr->simplify(), falseExpr->simplify());
if (!v) {
return std::make_unique<ConditionalExpression>(
owner,
condition->simplify().release(),
trueExpr->simplify().release(),
falseExpr->simplify().release()
);
}
else {
if (fabs(v->getValue()) >= Base::Precision::Confusion())
if (fabs(v->getValue()) >= Base::Precision::Confusion()) {
return trueExpr->simplify();
else
}
else {
return falseExpr->simplify();
}
}
}
@@ -3270,9 +3281,9 @@ Expression *RangeExpression::_copy() const
return new RangeExpression(owner, begin, end);
}
Expression *RangeExpression::simplify() const
ExpressionPtr RangeExpression::simplify() const
{
return copy().release();
return copy();
}
void RangeExpression::_getIdentifiers(std::map<App::ObjectIdentifier,bool> &deps) const
@@ -3714,7 +3725,7 @@ std::unique_ptr<UnitExpression> ExpressionParser::parseUnit(
throw ParserError("Unknown error in expression");
// Simplify expression
Expression * simplified = ScanResult->simplify();
ExpressionPtr simplified = ScanResult->simplify();
if (!unitExpression) {
auto* fraction = freecad_cast<OperatorExpression*>(ScanResult.get());
@@ -3730,19 +3741,14 @@ std::unique_ptr<UnitExpression> ExpressionParser::parseUnit(
}
ScanResult.reset();
if (unitExpression) {
NumberExpression * num = freecad_cast<NumberExpression*>(simplified);
if (num) {
simplified = new UnitExpression(num->getOwner(), num->getQuantity());
delete num;
}
return std::unique_ptr<UnitExpression>(freecad_cast<UnitExpression*>(simplified));
}
else {
delete simplified;
if (!unitExpression) {
throw Expression::Exception("Expression is not a unit.");
}
if (auto num = freecad_cast<NumberExpression*>(simplified.get()); num) {
return std::make_unique<UnitExpression>(num->getOwner(), num->getQuantity());
}
return std::unique_ptr<UnitExpression>(freecad_cast<UnitExpression*>(simplified.release()));
}
namespace {
+1 -1
View File
@@ -487,7 +487,7 @@ public:
*
* @return The simplified expression.
*/
virtual Expression * simplify() const = 0;
virtual ExpressionPtr simplify() const = 0;
/// Visit the expression with a visitor.
void visit(ExpressionVisitor & v);
+10 -10
View File
@@ -88,7 +88,7 @@ public:
~UnitExpression() override;
Expression* simplify() const override;
ExpressionPtr simplify() const override;
void setUnit(const Base::Quantity& _quantity);
@@ -144,7 +144,7 @@ public:
explicit NumberExpression(const App::DocumentObject* _owner = nullptr,
const Base::Quantity& quantity = Base::Quantity());
Expression* simplify() const override;
ExpressionPtr simplify() const override;
/**
* @brief Negate the stored value.
@@ -221,7 +221,7 @@ public:
bool isTouched() const override;
Expression* simplify() const override;
ExpressionPtr simplify() const override;
int priority() const override;
@@ -274,7 +274,7 @@ public:
bool isTouched() const override;
Expression* simplify() const override;
ExpressionPtr simplify() const override;
int priority() const override;
@@ -403,7 +403,7 @@ public:
bool isTouched() const override;
Expression* simplify() const override;
ExpressionPtr simplify() const override;
static Py::Object
evaluate(const Expression* owner, int type, const std::vector<Expression*>& args);
@@ -465,7 +465,7 @@ public:
bool isTouched() const override;
Expression* simplify() const override;
ExpressionPtr simplify() const override;
std::string name() const
{
@@ -540,9 +540,9 @@ public:
void setPyValue(Py::Object pyobj);
void setPyValue(PyObject* pyobj, bool owned = false);
Expression* simplify() const override
ExpressionPtr simplify() const override
{
return copy().release();
return copy();
}
protected:
@@ -568,7 +568,7 @@ public:
const std::string& _text = std::string());
~StringExpression() override;
Expression* simplify() const override;
ExpressionPtr simplify() const override;
virtual std::string getText() const
{
@@ -602,7 +602,7 @@ public:
bool isTouched() const override;
App::Expression* simplify() const override;
ExpressionPtr simplify() const override;
Range getRange() const;
+3 -3
View File
@@ -372,7 +372,7 @@ TEST_F(Evaluate, test_evaluate_simple)
TEST_F(Evaluate, test_simplify_simple)
{
App::ExpressionPtr e = App::ExpressionParser::parse(this_obj(), "1 + 2");
App::Expression* simplified = e->simplify();
App::ExpressionPtr simplified = e->simplify();
EXPECT_EQ(e->toString(), "1 + 2");
EXPECT_EQ(simplified->toString(), "3");
}
@@ -388,7 +388,7 @@ TEST_F(Evaluate, test_evaluate_function)
TEST_F(Evaluate, test_simplify_function)
{
App::ExpressionPtr e = App::ExpressionParser::parse(this_obj(), "sqrt(4)");
App::Expression* simplified = e->simplify();
App::ExpressionPtr simplified = e->simplify();
EXPECT_EQ(e->toString(), "sqrt(4)");
EXPECT_EQ(simplified->toString(), "2");
}
@@ -408,7 +408,7 @@ TEST_F(Evaluate, test_simplify_refer_to_var)
auto* prop = freecad_cast<App::PropertyFloat*>(this_obj()->addDynamicProperty("App::PropertyFloat", "Var"));
prop->setValue(2.0);
App::ExpressionPtr e = App::ExpressionParser::parse(this_obj(), "sqrt(2 + Var)");
App::Expression* simplified = e->simplify();
App::ExpressionPtr simplified = e->simplify();
EXPECT_EQ(e->toString(), "sqrt(2 + Var)");
EXPECT_EQ(simplified->toString(), "sqrt(2 + Var)");
}