Merge pull request #22788 from pieterhijma/fix-func-expr-simplify

Core: Fix expression function simplify

src/App/Expression.cpp

@@ -2665,7 +2665,7 @@ Py::Object FunctionExpression::_getPyValue() const {
 Expression *FunctionExpression::simplify() const
 {
     size_t numerics = 0;
-    std::vector<Expression*> a;
+    std::vector<Expression*> simplifiedArgs;
 
     // Try to simplify each argument to function
     for (auto it : args) {
@@ -2673,20 +2673,21 @@ Expression *FunctionExpression::simplify() const
 
         if (freecad_cast<NumberExpression*>(v))
             ++numerics;
-        a.push_back(v);
+        simplifiedArgs.push_back(v);
     }
 
     if (numerics == args.size()) {
         // All constants, then evaluation must also be constant
 
-        // Clean-up
-        for (auto it : args)
+        // Clean-up the simplified arguments
+        for (auto it : simplifiedArgs)
             delete it;
 
         return eval();
     }
     else
-        return new FunctionExpression(owner, f, std::string(fname), std::move(a));
+        return new FunctionExpression(owner, f, std::string(fname),
+                                      std::move(simplifiedArgs));
 }
 
 /**

tests/src/App/Expression.cpp

@@ -1,7 +1,33 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/***************************************************************************
+ *   Copyright (c) 2023 Werner Mayer <wmayer[at]users.sourceforge.net>     *
+ *   Copyright (c) 2025 Pieter Hijma <info@pieterhijma.net>                 *
+ *                                                                         *
+ *   This file is part of FreeCAD.                                         *
+ *                                                                         *
+ *   FreeCAD is free software: you can redistribute it and/or modify it    *
+ *   under the terms of the GNU Lesser General Public License as           *
+ *   published by the Free Software Foundation, either version 2.1 of the  *
+ *   License, or (at your option) any later version.                       *
+ *                                                                         *
+ *   FreeCAD is distributed in the hope that it will be useful, but        *
+ *   WITHOUT ANY WARRANTY; without even the implied warranty of            *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU      *
+ *   Lesser General Public License for more details.                       *
+ *                                                                         *
+ *   You should have received a copy of the GNU Lesser General Public      *
+ *   License along with FreeCAD. If not, see                               *
+ *   <https://www.gnu.org/licenses/>.                                      *
+ *                                                                         *
+ **************************************************************************/
+
 #include <gtest/gtest.h>
 
 #include <src/App/InitApplication.h>
 
+#include "App/Document.h"
+#include "App/DocumentObject.h"
+#include "App/Expression.h"
 #include "App/ExpressionParser.h"
 #include "App/ExpressionTokenizer.h"
 
@@ -304,4 +330,86 @@ TEST_F(Expression, test_e_rad)
     EXPECT_EQ(op->toString(), "e rad");
     op.release();
 }
+
+class Evaluate: public ::testing::Test
+{
+protected:
+    static void SetUpTestSuite()
+    {
+        tests::initApplication();
+    }
+
+    void SetUp() override
+    {
+        _doc_name = App::GetApplication().getUniqueDocumentName("test");
+        _this_doc = App::GetApplication().newDocument(_doc_name.c_str(), "testUser");
+        _this_obj = _this_doc->addObject("App::VarSet");
+    }
+
+    void TearDown() override
+    {
+        App::GetApplication().closeDocument(_doc_name.c_str());
+    }
+
+    App::DocumentObject* this_obj() { return _this_obj; }
+
+private:
+    std::string _doc_name;
+    App::Document* _this_doc {};
+    App::DocumentObject* _this_obj {};
+};
+
+// Various test for evaluating and simplifying expressions
+// Each "evaluate" test has an equivalent "simplify" test
+TEST_F(Evaluate, test_evaluate_simple)
+{
+    App::Expression* e = App::ExpressionParser::parse(this_obj(), "1 + 2");
+    App::Expression* evaluated = e->eval();
+    EXPECT_EQ(e->toString(), "1 + 2");
+    EXPECT_EQ(evaluated->toString(), "3");
+}
+
+TEST_F(Evaluate, test_simplify_simple)
+{
+    App::Expression* e = App::ExpressionParser::parse(this_obj(), "1 + 2");
+    App::Expression* simplified = e->simplify();
+    EXPECT_EQ(e->toString(), "1 + 2");
+    EXPECT_EQ(simplified->toString(), "3");
+}
+
+TEST_F(Evaluate, test_evaluate_function)
+{
+    App::Expression* e = App::ExpressionParser::parse(this_obj(), "sqrt(4)");
+    App::Expression* evaluated = e->eval();
+    EXPECT_EQ(e->toString(), "sqrt(4)");
+    EXPECT_EQ(evaluated->toString(), "2");
+}
+
+TEST_F(Evaluate, test_simplify_function)
+{
+    App::Expression* e = App::ExpressionParser::parse(this_obj(), "sqrt(4)");
+    App::Expression* simplified = e->simplify();
+    EXPECT_EQ(e->toString(), "sqrt(4)");
+    EXPECT_EQ(simplified->toString(), "2");
+}
+
+TEST_F(Evaluate, test_evaluate_refer_to_var)
+{
+    auto* prop = freecad_cast<App::PropertyFloat*>(this_obj()->addDynamicProperty("App::PropertyFloat", "Var"));
+    prop->setValue(2.0);
+    App::Expression* e = App::ExpressionParser::parse(this_obj(), "sqrt(2 + Var)");
+    App::Expression* evaluated = e->eval();
+    EXPECT_EQ(e->toString(), "sqrt(2 + Var)");
+    EXPECT_EQ(evaluated->toString(), "2");
+}
+
+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::Expression* e = App::ExpressionParser::parse(this_obj(), "sqrt(2 + Var)");
+    App::Expression* simplified = e->simplify();
+    EXPECT_EQ(e->toString(), "sqrt(2 + Var)");
+    EXPECT_EQ(simplified->toString(), "sqrt(2 + Var)");
+}
 // clang-format on