make error() propagate from custom replace functions

it didn't, which is rather unexpected.

Change-Id: I8cdb7b1490a8c2207809812b93cc65fbe23a1b98
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

4 files changed, 144 insertions, 55 deletions

diff --git a/qmake/library/qmakeevaluator.cpp b/qmake/library/qmakeevaluator.cpp
index 30f8619bc1..1243a77afa 100644
--- a/qmake/library/qmakeevaluator.cpp
+++ b/qmake/library/qmakeevaluator.cpp
@@ -409,7 +409,7 @@ static ALWAYS_INLINE void addStrList(
     }
 }
 
-void QMakeEvaluator::evaluateExpression(
+QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateExpression(
         const ushort *&tokPtr, ProStringList *ret, bool joined)
 {
     debugMsg(2, joined ? "evaluating joined expression" : "evaluating expression");
@@ -459,12 +459,15 @@ void QMakeEvaluator::evaluateExpression(
         case TokFuncName: {
             const ProKey &func = pro->getHashStr(tokPtr);
             debugMsg(2, "function %s", dbgKey(func));
-            addStrList(evaluateExpandFunction(func, tokPtr), tok, ret, pending, joined);
+            ProStringList val;
+            if (evaluateExpandFunction(func, tokPtr, &val) == ReturnError)
+                return ReturnError;
+            addStrList(val, tok, ret, pending, joined);
             break; }
         default:
             debugMsg(2, "evaluated expression => %s", dbgStrList(*ret));
             tokPtr--;
-            return;
+            return ReturnTrue;
         }
     }
 }
@@ -536,7 +539,9 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::visitProBlock(
         case TokAppendUnique:
         case TokRemove:
         case TokReplace:
-            visitProVariable(tok, curr, tokPtr);
+            ret = visitProVariable(tok, curr, tokPtr);
+            if (ret == ReturnError)
+                break;
             curr.clear();
             continue;
         case TokBranch:
@@ -696,9 +701,9 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::visitProBlock(
             continue;
         default: {
                 const ushort *oTokPtr = --tokPtr;
-                evaluateExpression(tokPtr, &curr, false);
-                if (tokPtr != oTokPtr)
-                    continue;
+                ret = evaluateExpression(tokPtr, &curr, false);
+                if (ret == ReturnError || tokPtr != oTokPtr)
+                    break;
             }
             Q_ASSERT_X(false, "visitProBlock", "unexpected item type");
             continue;
@@ -731,7 +736,10 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::visitProLoop(
     int index = 0;
     ProKey variable;
     ProStringList oldVarVal;
-    ProString it_list = expandVariableReferences(exprPtr, 0, true).at(0);
+    ProStringList it_list_out;
+    if (expandVariableReferences(exprPtr, 0, &it_list_out, true) == ReturnError)
+        return ReturnError;
+    ProString it_list = it_list_out.at(0);
     if (_variable.isEmpty()) {
         if (it_list != statics.strever) {
             evalError(fL1S("Invalid loop expression."));
@@ -828,7 +836,7 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::visitProLoop(
     return ret;
 }
 
-void QMakeEvaluator::visitProVariable(
+QMakeEvaluator::VisitReturn QMakeEvaluator::visitProVariable(
         ushort tok, const ProStringList &curr, const ushort *&tokPtr)
 {
     int sizeHint = *tokPtr++;
@@ -837,24 +845,26 @@ void QMakeEvaluator::visitProVariable(
         skipExpression(tokPtr);
         if (!m_cumulative || !curr.isEmpty())
             evalError(fL1S("Left hand side of assignment must expand to exactly one word."));
-        return;
+        return ReturnTrue;
     }
     const ProKey &varName = map(curr.first());
 
     if (tok == TokReplace) {      // ~=
         // DEFINES ~= s/a/b/?[gqi]
 
-        const ProStringList &varVal = expandVariableReferences(tokPtr, sizeHint, true);
+        ProStringList varVal;
+        if (expandVariableReferences(tokPtr, sizeHint, &varVal, true) == ReturnError)
+            return ReturnError;
         const QString &val = varVal.at(0).toQString(m_tmp1);
         if (val.length() < 4 || val.at(0) != QLatin1Char('s')) {
             evalError(fL1S("The ~= operator can handle only the s/// function."));
-            return;
+            return ReturnTrue;
         }
         QChar sep = val.at(1);
         QStringList func = val.split(sep);
         if (func.count() < 3 || func.count() > 4) {
             evalError(fL1S("The s/// function expects 3 or 4 arguments."));
-            return;
+            return ReturnTrue;
         }
 
         bool global = false, quote = false, case_sense = false;
@@ -875,7 +885,9 @@ void QMakeEvaluator::visitProVariable(
         replaceInList(&valuesRef(varName), regexp, replace, global, m_tmp2);
         debugMsg(2, "replaced %s with %s", dbgQStr(pattern), dbgQStr(replace));
     } else {
-        ProStringList varVal = expandVariableReferences(tokPtr, sizeHint);
+        ProStringList varVal;
+        if (expandVariableReferences(tokPtr, sizeHint, &varVal, false) == ReturnError)
+            return ReturnError;
         switch (tok) {
         default: // whatever - cannot happen
         case TokAssign:          // =
@@ -923,6 +935,8 @@ void QMakeEvaluator::visitProVariable(
     else if (varName == statics.strREQUIRES)
         checkRequirements(values(varName));
 #endif
+
+    return ReturnTrue;
 }
 
 void QMakeEvaluator::setTemplate()
@@ -1610,18 +1624,18 @@ bool QMakeEvaluator::isActiveConfig(const QString &config, bool regex)
     return false;
 }
 
-ProStringList QMakeEvaluator::expandVariableReferences(
-        const ushort *&tokPtr, int sizeHint, bool joined)
+QMakeEvaluator::VisitReturn QMakeEvaluator::expandVariableReferences(
+        const ushort *&tokPtr, int sizeHint, ProStringList *ret, bool joined)
 {
-    ProStringList ret;
-    ret.reserve(sizeHint);
+    ret->reserve(sizeHint);
     forever {
-        evaluateExpression(tokPtr, &ret, joined);
+        if (evaluateExpression(tokPtr, ret, joined) == ReturnError)
+            return ReturnError;
         switch (*tokPtr) {
         case TokValueTerminator:
         case TokFuncTerminator:
             tokPtr++;
-            return ret;
+            return ReturnTrue;
         case TokArgSeparator:
             if (joined) {
                 tokPtr++;
@@ -1635,28 +1649,28 @@ ProStringList QMakeEvaluator::expandVariableReferences(
     }
 }
 
-QList<ProStringList> QMakeEvaluator::prepareFunctionArgs(const ushort *&tokPtr)
+QMakeEvaluator::VisitReturn QMakeEvaluator::prepareFunctionArgs(
+        const ushort *&tokPtr, QList<ProStringList> *ret)
 {
-    QList<ProStringList> args_list;
     if (*tokPtr != TokFuncTerminator) {
         for (;; tokPtr++) {
             ProStringList arg;
-            evaluateExpression(tokPtr, &arg, false);
-            args_list << arg;
+            if (evaluateExpression(tokPtr, &arg, false) == ReturnError)
+                return ReturnError;
+            *ret << arg;
             if (*tokPtr == TokFuncTerminator)
                 break;
             Q_ASSERT(*tokPtr == TokArgSeparator);
         }
     }
     tokPtr++;
-    return args_list;
+    return ReturnTrue;
 }
 
-ProStringList QMakeEvaluator::evaluateFunction(
-        const ProFunctionDef &func, const QList<ProStringList> &argumentsList, VisitReturn *ok)
+QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateFunction(
+        const ProFunctionDef &func, const QList<ProStringList> &argumentsList, ProStringList *ret)
 {
     VisitReturn vr;
-    ProStringList ret;
 
     if (m_valuemapStack.count() >= 100) {
         evalError(fL1S("Ran into infinite recursion (depth > 100)."));
@@ -1675,25 +1689,22 @@ ProStringList QMakeEvaluator::evaluateFunction(
         vr = visitProBlock(func.pro(), func.tokPtr());
         if (vr == ReturnReturn)
             vr = ReturnTrue;
-        ret = m_returnValue;
+        if (vr == ReturnTrue)
+            *ret = m_returnValue;
         m_returnValue.clear();
 
         m_current = m_locationStack.pop();
         m_valuemapStack.pop();
     }
-    if (ok)
-        *ok = vr;
-    if (vr == ReturnTrue)
-        return ret;
-    return ProStringList();
+    return vr;
 }
 
 QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateBoolFunction(
         const ProFunctionDef &func, const QList<ProStringList> &argumentsList,
         const ProString &function)
 {
-    VisitReturn vr;
-    ProStringList ret = evaluateFunction(func, argumentsList, &vr);
+    ProStringList ret;
+    VisitReturn vr = evaluateFunction(func, argumentsList, &ret);
     if (vr == ReturnTrue) {
         if (ret.isEmpty())
             return ReturnTrue;
@@ -1721,13 +1732,18 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateConditionalFunction(
 {
     if (int func_t = statics.functions.value(func)) {
         //why don't the builtin functions just use args_list? --Sam
-        return evaluateBuiltinConditional(func_t, func, expandVariableReferences(tokPtr, 5, true));
+        ProStringList args;
+        if (expandVariableReferences(tokPtr, 5, &args, true) == ReturnError)
+            return ReturnError;
+        return evaluateBuiltinConditional(func_t, func, args);
     }
 
     QHash<ProKey, ProFunctionDef>::ConstIterator it =
             m_functionDefs.testFunctions.constFind(func);
     if (it != m_functionDefs.testFunctions.constEnd()) {
-        const QList<ProStringList> args = prepareFunctionArgs(tokPtr);
+        QList<ProStringList> args;
+        if (prepareFunctionArgs(tokPtr, &args) == ReturnError)
+            return ReturnError;
         traceMsg("calling %s(%s)", dbgKey(func), dbgStrListList(args));
         return evaluateBoolFunction(*it, args, func);
     }
@@ -1737,25 +1753,31 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateConditionalFunction(
     return ReturnFalse;
 }
 
-ProStringList QMakeEvaluator::evaluateExpandFunction(
-        const ProKey &func, const ushort *&tokPtr)
+QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateExpandFunction(
+        const ProKey &func, const ushort *&tokPtr, ProStringList *ret)
 {
     if (int func_t = statics.expands.value(func)) {
         //why don't the builtin functions just use args_list? --Sam
-        return evaluateBuiltinExpand(func_t, func, expandVariableReferences(tokPtr, 5, true));
+        ProStringList args;
+        if (expandVariableReferences(tokPtr, 5, &args, true) == ReturnError)
+            return ReturnError;
+        *ret = evaluateBuiltinExpand(func_t, func, args);
+        return ReturnTrue;
     }
 
     QHash<ProKey, ProFunctionDef>::ConstIterator it =
             m_functionDefs.replaceFunctions.constFind(func);
     if (it != m_functionDefs.replaceFunctions.constEnd()) {
-        const QList<ProStringList> args = prepareFunctionArgs(tokPtr);
+        QList<ProStringList> args;
+        if (prepareFunctionArgs(tokPtr, &args) == ReturnError)
+            return ReturnError;
         traceMsg("calling $$%s(%s)", dbgKey(func), dbgStrListList(args));
-        return evaluateFunction(*it, args, 0);
+        return evaluateFunction(*it, args, ret);
     }
 
     skipExpression(tokPtr);
     evalError(fL1S("'%1' is not a recognized replace function.").arg(func.toQString(m_tmp1)));
-    return ProStringList();
+    return ReturnFalse;
 }
 
 QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateConditional(
diff --git a/qmake/library/qmakeevaluator.h b/qmake/library/qmakeevaluator.h
index 7db07b17ba..9625c8b8df 100644
--- a/qmake/library/qmakeevaluator.h
+++ b/qmake/library/qmakeevaluator.h
@@ -153,7 +153,7 @@ public:
         { return b ? ReturnTrue : ReturnFalse; }
 
     static ALWAYS_INLINE uint getBlockLen(const ushort *&tokPtr);
-    void evaluateExpression(const ushort *&tokPtr, ProStringList *ret, bool joined);
+    VisitReturn evaluateExpression(const ushort *&tokPtr, ProStringList *ret, bool joined);
     static ALWAYS_INLINE void skipStr(const ushort *&tokPtr);
     static ALWAYS_INLINE void skipHashStr(const ushort *&tokPtr);
     void skipExpression(const ushort *&tokPtr);
@@ -173,7 +173,7 @@ public:
     VisitReturn visitProLoop(const ProKey &variable, const ushort *exprPtr,
                              const ushort *tokPtr);
     void visitProFunctionDef(ushort tok, const ProKey &name, const ushort *tokPtr);
-    void visitProVariable(ushort tok, const ProStringList &curr, const ushort *&tokPtr);
+    VisitReturn visitProVariable(ushort tok, const ProStringList &curr, const ushort *&tokPtr);
 
     ALWAYS_INLINE const ProKey &map(const ProString &var) { return map(var.toKey()); }
     const ProKey &map(const ProKey &var);
@@ -182,7 +182,7 @@ public:
     void setTemplate();
 
     ProStringList split_value_list(const QString &vals, const ProFile *source = 0);
-    ProStringList expandVariableReferences(const ushort *&tokPtr, int sizeHint = 0, bool joined = false);
+    VisitReturn expandVariableReferences(const ushort *&tokPtr, int sizeHint, ProStringList *ret, bool joined);
 
     QString currentFileName() const;
     QString currentDirectory() const;
@@ -207,14 +207,14 @@ public:
     void deprecationWarning(const QString &msg) const
             { message(QMakeHandler::EvalWarnDeprecated, msg); }
 
-    QList<ProStringList> prepareFunctionArgs(const ushort *&tokPtr);
-    ProStringList evaluateFunction(const ProFunctionDef &func,
-                                   const QList<ProStringList> &argumentsList, VisitReturn *ok);
+    VisitReturn prepareFunctionArgs(const ushort *&tokPtr, QList<ProStringList> *ret);
+    VisitReturn evaluateFunction(const ProFunctionDef &func,
+                                 const QList<ProStringList> &argumentsList, ProStringList *ret);
     VisitReturn evaluateBoolFunction(const ProFunctionDef &func,
                                      const QList<ProStringList> &argumentsList,
                                      const ProString &function);
 
-    ProStringList evaluateExpandFunction(const ProKey &function, const ushort *&tokPtr);
+    VisitReturn evaluateExpandFunction(const ProKey &function, const ushort *&tokPtr, ProStringList *ret);
     VisitReturn evaluateConditionalFunction(const ProKey &function, const ushort *&tokPtr);
 
     ProStringList evaluateBuiltinExpand(int func_t, const ProKey &function, const ProStringList &args);
diff --git a/qmake/project.cpp b/qmake/project.cpp
index faea6da650..9c06f7ef5d 100644
--- a/qmake/project.cpp
+++ b/qmake/project.cpp
@@ -110,9 +110,8 @@ QStringList QMakeProject::expand(const ProKey &func, const QList<ProStringList>
     QHash<ProKey, ProFunctionDef>::ConstIterator it =
             m_functionDefs.replaceFunctions.constFind(func);
     if (it != m_functionDefs.replaceFunctions.constEnd()) {
-        QMakeProject::VisitReturn vr;
-        ProStringList ret = evaluateFunction(*it, args, &vr);
-        if (vr == QMakeProject::ReturnError)
+        ProStringList ret;
+        if (evaluateFunction(*it, args, &ret) == QMakeProject::ReturnError)
             exit(3);
         return ret.toQStringList();
     }
@@ -130,7 +129,9 @@ ProString QMakeProject::expand(const QString &expr, const QString &where, int li
         m_current.pro = pro;
         m_current.line = 0;
         const ushort *tokPtr = pro->tokPtr();
-        ProStringList result = expandVariableReferences(tokPtr, 1, true);
+        ProStringList result;
+        if (expandVariableReferences(tokPtr, 1, &result, true) == ReturnError)
+            exit(3);
         if (!result.isEmpty())
             ret = result.at(0);
     }
diff --git a/tests/auto/tools/qmakelib/evaltest.cpp b/tests/auto/tools/qmakelib/evaltest.cpp
index f172cda865..b7acf70e88 100644
--- a/tests/auto/tools/qmakelib/evaltest.cpp
+++ b/tests/auto/tools/qmakelib/evaltest.cpp
@@ -641,6 +641,72 @@ void tst_qmakelib::addControlStructs()
             << "VAR = final"
             << ""
             << true;
+
+    QTest::newRow("error() from replace function (assignment)")
+            << "defineReplace(func) {\nerror(error)\n}\n"
+               "VAR = $$func()\n"
+               "OKE = 1"
+            << "VAR = UNDEF\nOKE = UNDEF"
+            << "Project ERROR: error"
+            << false;
+
+    QTest::newRow("error() from replace function (replacement)")
+            << "defineReplace(func) {\nerror(error)\n}\n"
+               "VAR = $$func()\n"
+               "OKE = 1"
+            << "VAR = UNDEF\nOKE = UNDEF"
+            << "Project ERROR: error"
+            << false;
+
+    QTest::newRow("error() from replace function (LHS)")
+            << "defineReplace(func) {\nerror(error)\nreturn(VAR)\n}\n"
+               "$$func() = 1\n"
+               "OKE = 1"
+            << "VAR = UNDEF\nOKE = UNDEF"
+            << "Project ERROR: error"
+            << false;
+
+    QTest::newRow("error() from replace function (loop variable)")
+            << "defineReplace(func) {\nerror(error)\nreturn(BLAH)\n}\n"
+               "for($$func()) {\nVAR = $$BLAH\nbreak()\n}\n"
+               "OKE = 1"
+            << "VAR = UNDEF\nOKE = UNDEF"
+            << "Project ERROR: error"
+            << false;
+
+    QTest::newRow("error() from replace function (built-in test arguments)")
+            << "defineReplace(func) {\nerror(error)\n}\n"
+               "message($$func()): VAR = 1\n"
+               "OKE = 1"
+            << "VAR = UNDEF\nOKE = UNDEF"
+            << "Project ERROR: error"
+            << false;
+
+    QTest::newRow("error() from replace function (built-in replace arguments)")
+            << "defineReplace(func) {\nerror(error)\n}\n"
+               "VAR = $$upper($$func())\n"
+               "OKE = 1"
+            << "VAR = UNDEF\nOKE = UNDEF"
+            << "Project ERROR: error"
+            << false;
+
+    QTest::newRow("error() from replace function (custom test arguments)")
+            << "defineReplace(func) {\nerror(error)\n}\n"
+               "defineTest(custom) {\n}\n"
+               "custom($$func()): VAR = 1\n"
+               "OKE = 1"
+            << "VAR = UNDEF\nOKE = UNDEF"
+            << "Project ERROR: error"
+            << false;
+
+    QTest::newRow("error() from replace function (custom replace arguments)")
+            << "defineReplace(func) {\nerror(error)\nreturn(1)\n}\n"
+               "defineReplace(custom) {\nreturn($$1)\n}\n"
+               "VAR = $$custom($$func(1))\n"
+               "OKE = 1"
+            << "VAR = UNDEF\nOKE = UNDEF"
+            << "Project ERROR: error"
+            << false;
 }
 
 void tst_qmakelib::addReplaceFunctions(const QString &qindir)