Fixed|Scripting|libcore: Fixed a memory leak when evaluating and/or

OperatorExpression was leaking the left operand’s result value.

doomsday/sdk/libcore/src/scriptsys/evaluator.cpp

@@ -61,7 +61,7 @@ DENG2_PIMPL(Evaluator)
     /// Namespace for the current expression.
     Record *names;
 
-    Expressions stack;
+    Expressions expressions;
     Results results;
 
     /// Returned when there is no result to give.
@@ -76,7 +76,7 @@ DENG2_PIMPL(Evaluator)
 
     ~Impl()
     {
-        DENG2_ASSERT(stack.isEmpty());
+        DENG2_ASSERT(expressions.isEmpty());
         clearNames();
         clearResults();
     }
@@ -99,11 +99,11 @@ DENG2_PIMPL(Evaluator)
         results.clear();
     }
 
-    void clearStack()
+    void clearExpressions()
     {
-        while (!stack.empty())
+        while (!expressions.empty())
         {
-            ScopedExpression top = stack.takeLast();
+            ScopedExpression top = expressions.takeLast();
             clearNames();
             names = top.names();
             delete top.scope;
@@ -112,12 +112,13 @@ DENG2_PIMPL(Evaluator)
 
     void pushResult(Value *value, Value *scope = 0 /*take*/)
     {
-        // NULLs are not pushed onto the results stack as they indicate that
+        // NULLs are not pushed onto the results expressions as they indicate that
         // no result was given.
         if (value)
         {
-            /*qDebug() << "Evaluator: Pushing result" << value->asText() << "in scope"
-                        << (scope? scope->asText() : "null");*/
+            /*qDebug() << "Evaluator: Pushing result" << value << value->asText() << "in scope"
+                        << (scope? scope->asText() : "null")
+                        << "result stack size:" << results.size();*/
             results << ScopedResult(value, scope);
         }
         else
@@ -137,8 +138,8 @@ DENG2_PIMPL(Evaluator)
 
     Value &evaluate(Expression const *expression)
     {
-        DENG2_ASSERT(names == NULL);
-        DENG2_ASSERT(stack.empty());
+        DENG2_ASSERT(names == nullptr);
+        DENG2_ASSERT(expressions.empty());
 
         //qDebug() << "Evaluator: Starting evaluation of" << expression;
 
@@ -149,10 +150,10 @@ DENG2_PIMPL(Evaluator)
         // Clear the result stack.
         clearResults();
 
-        while (!stack.empty())
+        while (!expressions.empty())
         {
             // Continue by processing the next step in the evaluation.
-            ScopedExpression top = stack.takeLast();
+            ScopedExpression top = expressions.takeLast();
             clearNames();
             names = top.names();
             /*qDebug() << "Evaluator: Evaluating latest scoped expression" << top.expression
@@ -169,7 +170,7 @@ DENG2_PIMPL(Evaluator)
         DENG2_ASSERT(self().hasResult());
 
         clearNames();
-        current = NULL;
+        current = nullptr;
         return result();
     }
 };
@@ -200,7 +201,7 @@ void Evaluator::reset()
 {
     d->current = nullptr;
 
-    d->clearStack();
+    d->clearExpressions();
     d->clearNames();
 }
 
@@ -245,7 +246,7 @@ Value &Evaluator::result()
 
 void Evaluator::push(Expression const *expression, Value *scope)
 {
-    d->stack.push_back(Impl::ScopedExpression(expression, scope));
+    d->expressions.push_back(Impl::ScopedExpression(expression, scope));
 }
 
 void Evaluator::pushResult(Value *value)
@@ -258,7 +259,7 @@ Value *Evaluator::popResult(Value **evaluationScope)
     DENG2_ASSERT(d->results.size() > 0);
 
     Impl::ScopedResult result = d->results.takeLast();
-    /*qDebug() << "Evaluator: Popping result" << result.result->asText()
+    /*qDebug() << "Evaluator: Popping result" << result.result << result.result->asText()
              << "in scope" << (result.scope? result.scope->asText() : "null");*/
 
     if (evaluationScope)

doomsday/sdk/libcore/src/scriptsys/operatorexpression.cpp

@@ -210,7 +210,7 @@ Value *OperatorExpression::evaluate(Evaluator &evaluator) const
             {
                 isResultTrue.push(evaluator);
                 _rightOperand->push(evaluator);
-                return nullptr;
+                result = nullptr;
             }
             break;
 
@@ -224,7 +224,7 @@ Value *OperatorExpression::evaluate(Evaluator &evaluator) const
             {
                 isResultTrue.push(evaluator);
                 _rightOperand->push(evaluator);
-                return nullptr;
+                result = nullptr;
             }
             break;
 
@@ -325,7 +325,7 @@ Value *OperatorExpression::evaluate(Evaluator &evaluator) const
 
     // Delete the unnecessary values.
     if (result != rightValue) delete rightValue;
-    if (result != leftValue) delete leftValue;
+    if (result != leftValue)  delete leftValue;
 
     return result;
 }

doomsday/sdk/libcore/src/scriptsys/process.cpp

@@ -321,13 +321,20 @@ void Process::finish(Value *returnValue)
         if (topmost->type() == Context::FunctionCall)
         {
             // Return value to the new topmost level.
+            //qDebug() << "Process: Pushing function return value" << returnValue << returnValue->asText();
             context().evaluator().pushResult(returnValue? returnValue : new NoneValue);
         }
+        else
+        {
+            DENG2_ASSERT(returnValue == nullptr);
+        }
     }
     else
     {
         DENG2_ASSERT(d->stack.back()->type() == Context::BaseProcess);
 
+        if (returnValue) delete returnValue; // Possible return value ignored.
+
         // This was the last level.
         d->state = Stopped;
     }
@@ -365,22 +372,18 @@ void Process::call(Function const &function, ArrayValue const &arguments, Value
             pushContext(new Context(Context::GlobalNamespace, this, function.globals()));
         }
 
-        // If the function is not in the global namespace, add its own parent
-        // namespace to the stack, too.
-
-
         // Create a new context.
         pushContext(new Context(Context::FunctionCall, this));
 
         // If the scope is defined, create the "self" variable for it.
         if (self)
         {
-            context().names().add(new Variable("self", self /*taken*/));
+            context().names().add(new Variable(QStringLiteral("self"), self /*taken*/));
         }
 
         // Create local variables for the arguments in the new context.
         Function::ArgumentValues::const_iterator b = argValues.begin();
-        Function::Arguments::const_iterator a = function.arguments().begin();
+        Function::Arguments     ::const_iterator a = function.arguments().begin();
         for (; b != argValues.end() && a != function.arguments().end(); ++b, ++a)
         {
             // Records must only be passed as unowned references.

doomsday/tests/test_script/main.cpp

@@ -34,6 +34,10 @@ int main(int argc, char **argv)
         app.initSubsystems(App::DisablePlugins);
 
         Script testScript(app.fileSystem().find("kitchen_sink.de"));
+#if 0
+        Script testScript("def returnValue(a): return a\n"
+                          "returnValue(True) and returnValue(True)\n");
+#endif
         Process proc(testScript);
         LOG_MSG("Script parsing is complete! Executing...");
         LOG_MSG("------------------------------------------------------------------------------");
@@ -49,5 +53,5 @@ int main(int argc, char **argv)
     }
 
     qDebug("Exiting main()...");
-    return 0;        
+    return 0;
 }