ExpressionParser: Return unique_ptr from parse (#27098)

- Start at ScanResult, as starting higher makes it easy to have memory
  issues.
- There are multiple memory leaks (at least in tests, but possibly in
  production code too) from using raw pointers in the public interface
  here. Fix these before adding more tests.
- Starts at ScanResult
- The bison changes were applied manually, as I don't think that it's
  pixified and running my local version showed many changes.
- I've chosen not to pimplify Expression, but only because it would be
  such a significant change for not so much value.

src/App/Expression.cpp

@@ -881,7 +881,7 @@ int Expression::priority() const {
     return 20;
 }
 
-Expression * Expression::parse(const DocumentObject *owner, const std::string &buffer)
+ExpressionPtr Expression::parse(const DocumentObject* owner, const std::string& buffer)
 {
     return ExpressionParser::parse(owner, buffer.c_str());
 }
@@ -3459,7 +3459,7 @@ double num_change(char* yytext,char dez_delim,char grp_delim)
 }
 
 /// The resulting expression after a successful parsing.
-static Expression* ScanResult = nullptr;
+static ExpressionPtr ScanResult = ExpressionPtr {};
 
 /// The DocumentObject that will own the expression.
 static const App::DocumentObject* DocumentObject = nullptr;
@@ -3539,7 +3539,7 @@ static void initParser(const App::DocumentObject *owner)
 
     using namespace App::ExpressionParser;
 
-    ScanResult = nullptr;
+    ScanResult.reset();
     App::ExpressionParser::DocumentObject = owner;
     labels = std::stack<std::string>();
     column = 0;
@@ -3659,7 +3659,7 @@ std::vector<std::tuple<int, int, std::string> > tokenize(const std::string &str)
   *
   */
 
-Expression * App::ExpressionParser::parse(const App::DocumentObject *owner, const char* buffer)
+ExpressionPtr App::ExpressionParser::parse(const App::DocumentObject* owner, const char* buffer)
 {
     // parse from buffer
     ExpressionParser::YY_BUFFER_STATE my_string_buffer = ExpressionParser::ExpressionParser_scan_string (buffer);
@@ -3678,16 +3678,17 @@ Expression * App::ExpressionParser::parse(const App::DocumentObject *owner, cons
         throw ParserError(fmt::format("Unknown error in expression '{}'", buffer));
     }
 
-    if (valueExpression) {
-        return ScanResult;
-    }
-    else {
-        delete ScanResult;
+    if (!valueExpression) {
+        ScanResult.reset();
         throw Expression::Exception("Expression can not evaluate to a value.");
     }
+    return std::exchange(ScanResult, nullptr);
 }
 
-UnitExpression * ExpressionParser::parseUnit(const App::DocumentObject *owner, const char* buffer)
+std::unique_ptr<UnitExpression> ExpressionParser::parseUnit(
+    const App::DocumentObject* owner,
+    const char* buffer
+)
 {
     // parse from buffer
     ExpressionParser::YY_BUFFER_STATE my_string_buffer = ExpressionParser::ExpressionParser_scan_string (buffer);
@@ -3708,7 +3709,7 @@ UnitExpression * ExpressionParser::parseUnit(const App::DocumentObject *owner, c
     Expression * simplified = ScanResult->simplify();
 
     if (!unitExpression) {
-        OperatorExpression * fraction = freecad_cast<OperatorExpression*>(ScanResult);
+        auto* fraction = freecad_cast<OperatorExpression*>(ScanResult.get());
 
         if (fraction && fraction->getOperator() == OperatorExpression::DIV) {
             NumberExpression * nom = freecad_cast<NumberExpression*>(fraction->getLeft());
@@ -3719,7 +3720,7 @@ UnitExpression * ExpressionParser::parseUnit(const App::DocumentObject *owner, c
                 unitExpression = true;
         }
     }
-    delete ScanResult;
+    ScanResult.reset();
 
     if (unitExpression) {
         NumberExpression * num = freecad_cast<NumberExpression*>(simplified);
@@ -3728,7 +3729,7 @@ UnitExpression * ExpressionParser::parseUnit(const App::DocumentObject *owner, c
            simplified = new UnitExpression(num->getOwner(), num->getQuantity());
             delete num;
         }
-        return freecad_cast<UnitExpression*>(simplified);
+        return std::unique_ptr<UnitExpression>(freecad_cast<UnitExpression*>(simplified));
     }
     else {
         delete simplified;

src/App/Expression.h

@@ -363,7 +363,7 @@ public:
      *
      * @return The parsed expression.
      */
-    static Expression* parse(const App::DocumentObject * owner, const std::string& buffer);
+    static ExpressionPtr parse(const App::DocumentObject* owner, const std::string& buffer);
 
     /// Copy an expression.
     Expression * copy() const;

src/App/Expression.tab.c

@@ -1314,13 +1314,13 @@ yyreduce:
     {
   case 2: /* input: exp  */
 #line 81 "Expression.y"
-                                                { ScanResult = (yyvsp[0].expr); valueExpression = true;                                        }
+                                                { ScanResult = std::unique_ptr<Expression>(yyvsp[0].expr); valueExpression = true;                                        }
 #line 1316 "Expression.tab.c"
     break;
 
   case 3: /* input: unit_exp  */
 #line 82 "Expression.y"
-                                                { ScanResult = (yyvsp[0].expr); unitExpression = true;                                         }
+                                                { ScanResult = std::unique_ptr<Expression>(yyvsp[0].expr); unitExpression = true;                                         }
 #line 1322 "Expression.tab.c"
     break;
 

src/App/Expression.y

@@ -80,8 +80,8 @@ std::stack<FunctionExpression::Function> functions;                /**< Function
 %%
 
 
-input:     exp                                  { ScanResult = $1; valueExpression = true;                                        }
-     |     unit_exp                             { ScanResult = $1; unitExpression = true;                                         }
+input:     exp                                  { ScanResult = std::unique_ptr<Expression>($1); valueExpression = true;                                        }
+     |     unit_exp                             { ScanResult = std::unique_ptr<Expression>($1); unitExpression = true;                                         }
      ;
 
 unit_num: num unit_exp %prec NUM_AND_UNIT       { $$ = new OperatorExpression(DocumentObject, $1, OperatorExpression::UNIT, $2);  }

src/App/ExpressionParser.h

@@ -630,8 +630,8 @@ protected:
  */
 namespace ExpressionParser
 {
-AppExport Expression* parse(const App::DocumentObject* owner, const char* buffer);
-AppExport UnitExpression* parseUnit(const App::DocumentObject* owner, const char* buffer);
+AppExport ExpressionPtr parse(const App::DocumentObject* owner, const char* buffer);
+AppExport std::unique_ptr<UnitExpression> parseUnit(const App::DocumentObject* owner, const char* buffer);
 AppExport ObjectIdentifier parsePath(const App::DocumentObject* owner, const char* buffer);
 AppExport bool isTokenAnIndentifier(const std::string& str);
 AppExport bool isTokenAConstant(const std::string& str);

src/Gui/Dialogs/DlgExpressionInput.cpp

@@ -669,7 +669,7 @@ void DlgExpressionInput::acceptWithVarSet()
 
     // Create a new expression that refers to the property in the VarSet
     // for the original property that is the target of this dialog.
-    expression.reset(ExpressionParser::parse(path.getDocumentObject(), prop->getFullName().c_str()));
+    expression = ExpressionParser::parse(path.getDocumentObject(), prop->getFullName().c_str());
 
     storePreferences(nameDoc.toStdString(), nameVarSet.toStdString(), group);
 }

src/Mod/Part/App/AttachExtension.cpp

@@ -608,7 +608,7 @@ void AttachExtension::handleLegacyTangentPlaneOrientation()
 
         // convert placement and expressions according to the dominant axis
         auto makeRotatedExpression =
-            [owner](const App::Expression* expr, double angle) -> App::Expression* {
+            [owner](const App::Expression* expr, double angle) -> App::ExpressionPtr {
             if (!expr) {
                 return nullptr;
             }
@@ -632,9 +632,9 @@ void AttachExtension::handleLegacyTangentPlaneOrientation()
 
             return App::ExpressionParser::parse(owner, unitSafeExprStr.c_str());
         };
-        App::Expression* newExprX = nullptr;
-        App::Expression* newExprY = nullptr;
-        App::Expression* newExprYaw = nullptr;
+        App::ExpressionPtr newExprX {};
+        App::ExpressionPtr newExprY {};
+        App::ExpressionPtr newExprYaw {};
         if (axis == 0) {  // normal mostly X
             // values
             std::swap(position.x, position.y);
@@ -689,9 +689,9 @@ void AttachExtension::handleLegacyTangentPlaneOrientation()
         // store updated placement and expressions back to the document object
 
         // expressions
-        owner->ExpressionEngine.setValue(oidX, App::ExpressionPtr(newExprX));
-        owner->ExpressionEngine.setValue(oidY, App::ExpressionPtr(newExprY));
-        owner->ExpressionEngine.setValue(oidYaw, App::ExpressionPtr(newExprYaw));
+        owner->ExpressionEngine.setValue(oidX, std::move(newExprX));
+        owner->ExpressionEngine.setValue(oidY, std::move(newExprY));
+        owner->ExpressionEngine.setValue(oidYaw, std::move(newExprYaw));
 
         // values
         placement.setPosition(position);

src/Mod/Spreadsheet/Gui/DlgSheetConf.cpp

@@ -112,7 +112,7 @@ App::Property* DlgSheetConf::prepare(
         std::string exprTxt(ui->lineEditProp->text().trimmed().toUtf8().constData());
         ExpressionPtr expr;
         try {
-            expr.reset(App::Expression::parse(sheet, exprTxt));
+            expr = App::Expression::parse(sheet, exprTxt);
         }
         catch (Base::Exception& e) {
             e.reportException();

tests/src/App/Expression.cpp

@@ -363,7 +363,7 @@ private:
 // 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::ExpressionPtr e = App::ExpressionParser::parse(this_obj(), "1 + 2");
     App::Expression* evaluated = e->eval();
     EXPECT_EQ(e->toString(), "1 + 2");
     EXPECT_EQ(evaluated->toString(), "3");
@@ -371,7 +371,7 @@ TEST_F(Evaluate, test_evaluate_simple)
 
 TEST_F(Evaluate, test_simplify_simple)
 {
-    App::Expression* e = App::ExpressionParser::parse(this_obj(), "1 + 2");
+    App::ExpressionPtr e = App::ExpressionParser::parse(this_obj(), "1 + 2");
     App::Expression* simplified = e->simplify();
     EXPECT_EQ(e->toString(), "1 + 2");
     EXPECT_EQ(simplified->toString(), "3");
@@ -379,7 +379,7 @@ TEST_F(Evaluate, test_simplify_simple)
 
 TEST_F(Evaluate, test_evaluate_function)
 {
-    App::Expression* e = App::ExpressionParser::parse(this_obj(), "sqrt(4)");
+    App::ExpressionPtr e = App::ExpressionParser::parse(this_obj(), "sqrt(4)");
     App::Expression* evaluated = e->eval();
     EXPECT_EQ(e->toString(), "sqrt(4)");
     EXPECT_EQ(evaluated->toString(), "2");
@@ -387,7 +387,7 @@ TEST_F(Evaluate, test_evaluate_function)
 
 TEST_F(Evaluate, test_simplify_function)
 {
-    App::Expression* e = App::ExpressionParser::parse(this_obj(), "sqrt(4)");
+    App::ExpressionPtr e = App::ExpressionParser::parse(this_obj(), "sqrt(4)");
     App::Expression* simplified = e->simplify();
     EXPECT_EQ(e->toString(), "sqrt(4)");
     EXPECT_EQ(simplified->toString(), "2");
@@ -397,7 +397,7 @@ 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::ExpressionPtr 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");
@@ -407,7 +407,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::Expression* e = App::ExpressionParser::parse(this_obj(), "sqrt(2 + Var)");
+    App::ExpressionPtr 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)");