don't pretend that break()/next()/return() are functions

it's a pretty braindead thing to implement control flow statements as
(built-in) functions.

as a "side effect", this fixes return() value handling for lists.

Change-Id: I59c8efa0e4d65329327115f7f8ed20719e7f7546
Reviewed-by: Daniel Teske <daniel.teske@nokia.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@nokia.com>

src/shared/proparser/proitems.h

@@ -286,6 +286,9 @@ enum ProToken {
     TokTestCall,        // previous literal/expansion is a test function call
                         // - ((nested expansion + TokArgSeparator)* + nested expansion)?
                         // - TokFuncTerminator
+    TokReturn,          // previous literal/expansion is a return value
+    TokBreak,           // break loop
+    TokNext,            // shortcut to next loop iteration
     TokNot,             // '!' operator
     TokAnd,             // ':' operator
     TokOr,              // '|' operator

src/shared/proparser/prowriter.cpp

@@ -159,6 +159,9 @@ static const ushort *skipToken(ushort tok, const ushort *&tokPtr, int &lineNo)
     case TokAnd:
     case TokOr:
     case TokCondition:
+    case TokReturn:
+    case TokNext:
+    case TokBreak:
         break;
     default: {
             const ushort *oTokPtr = --tokPtr;

src/shared/proparser/qmakebuiltins.cpp

@@ -85,7 +85,7 @@ enum ExpandFunc {
 enum TestFunc {
     T_INVALID = 0, T_REQUIRES, T_GREATERTHAN, T_LESSTHAN, T_EQUALS,
     T_EXISTS, T_EXPORT, T_CLEAR, T_UNSET, T_EVAL, T_CONFIG, T_SYSTEM,
-    T_RETURN, T_BREAK, T_NEXT, T_DEFINED, T_CONTAINS, T_INFILE,
+    T_DEFINED, T_CONTAINS, T_INFILE,
     T_COUNT, T_ISEMPTY, T_INCLUDE, T_LOAD, T_DEBUG, T_LOG, T_MESSAGE, T_WARNING, T_ERROR, T_IF,
     T_MKPATH, T_WRITE_FILE, T_TOUCH, T_CACHE
 };
@@ -157,9 +157,6 @@ void QMakeEvaluator::initFunctionStatics()
         { "if", T_IF },
         { "isActiveConfig", T_CONFIG },
         { "system", T_SYSTEM },
-        { "return", T_RETURN },
-        { "break", T_BREAK },
-        { "next", T_NEXT },
         { "defined", T_DEFINED },
         { "contains", T_CONTAINS },
         { "infile", T_INFILE },
@@ -1085,17 +1082,6 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateConditionalFunction(
         return returnBool(m_functionDefs.replaceFunctions.contains(var)
                           || m_functionDefs.testFunctions.contains(var));
     }
-    case T_RETURN:
-        m_returnValue = args;
-        // It is "safe" to ignore returns - due to qmake brokeness
-        // they cannot be used to terminate loops anyway.
-        if (m_cumulative)
-            return ReturnTrue;
-        if (m_valuemapStack.size() == 1) {
-            evalError(fL1S("unexpected return()."));
-            return ReturnFalse;
-        }
-        return ReturnReturn;
     case T_EXPORT: {
         if (args.count() != 1) {
             evalError(fL1S("export(variable) requires one argument."));
@@ -1165,20 +1151,6 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateConditionalFunction(
             }
             return ret;
         }
-    case T_BREAK:
-        if (m_skipLevel)
-            return ReturnFalse;
-        if (m_loopLevel)
-            return ReturnBreak;
-        evalError(fL1S("Unexpected break()."));
-        return ReturnFalse;
-    case T_NEXT:
-        if (m_skipLevel)
-            return ReturnFalse;
-        if (m_loopLevel)
-            return ReturnNext;
-        evalError(fL1S("Unexpected next()."));
-        return ReturnFalse;
     case T_IF: {
         if (args.count() != 1) {
             evalError(fL1S("if(condition) requires one argument."));

src/shared/proparser/qmakeevaluator.cpp

@@ -180,7 +180,6 @@ QMakeEvaluator::QMakeEvaluator(QMakeGlobals *option,
 #ifdef PROEVALUATOR_CUMULATIVE
     m_skipLevel = 0;
 #endif
-    m_loopLevel = 0;
     m_listCount = 0;
     m_valuemapStack.push(ProValueMap());
     m_valuemapInited = false;
@@ -630,6 +629,21 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::visitProBlock(
             invert = false;
             curr.clear();
             continue;
+        case TokReturn:
+            m_returnValue = curr;
+            curr.clear();
+            ret = ReturnReturn;
+            goto ctrlstm;
+        case TokBreak:
+            ret = ReturnBreak;
+            goto ctrlstm;
+        case TokNext:
+            ret = ReturnNext;
+          ctrlstm:
+            if (!m_skipLevel && okey != or_op)
+                return ret;
+            okey = false, or_op = true; // force next evaluation
+            continue;
         default: {
                 const ushort *oTokPtr = --tokPtr;
                 evaluateExpression(tokPtr, &curr, false);
@@ -701,7 +715,6 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::visitProLoop(
         }
     }
 
-    m_loopLevel++;
     forever {
         if (infinite) {
             if (!variable.isEmpty())
@@ -736,7 +749,6 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::visitProLoop(
         }
     }
   do_break:
-    m_loopLevel--;
 
     if (!variable.isEmpty())
         m_valuemapStack.top()[variable] = oldVarVal;
@@ -1477,8 +1489,6 @@ ProStringList QMakeEvaluator::evaluateFunction(
     } else {
         m_valuemapStack.push(ProValueMap());
         m_locationStack.push(m_current);
-        int loopLevel = m_loopLevel;
-        m_loopLevel = 0;
 
         ProStringList args;
         for (int i = 0; i < argumentsList.count(); ++i) {
@@ -1491,7 +1501,6 @@ ProStringList QMakeEvaluator::evaluateFunction(
         ret = m_returnValue;
         m_returnValue.clear();
 
-        m_loopLevel = loopLevel;
         m_current = m_locationStack.pop();
         m_valuemapStack.pop();
     }

src/shared/proparser/qmakeevaluator.h

@@ -197,7 +197,6 @@ public:
     static void removeEach(ProStringList *varlist, const ProStringList &value);
 
     QMakeEvaluator *m_caller;
-    int m_loopLevel; // To report unexpected break() and next()s
 #ifdef PROEVALUATOR_CUMULATIVE
     bool m_cumulative;
     int m_skipLevel;

src/shared/proparser/qmakeparser.cpp

@@ -97,6 +97,9 @@ static struct {
     QString strdefineTest;
     QString strdefineReplace;
     QString stroption;
+    QString strreturn;
+    QString strnext;
+    QString strbreak;
     QString strhost_build;
     QString strLINE;
     QString strFILE;
@@ -117,6 +120,9 @@ void QMakeParser::initialize()
     statics.strdefineTest = QLatin1String("defineTest");
     statics.strdefineReplace = QLatin1String("defineReplace");
     statics.stroption = QLatin1String("option");
+    statics.strreturn = QLatin1String("return");
+    statics.strnext = QLatin1String("next");
+    statics.strbreak = QLatin1String("break");
     statics.strhost_build = QLatin1String("host_build");
     statics.strLINE = QLatin1String("_LINE_");
     statics.strFILE = QLatin1String("_FILE_");
@@ -836,9 +842,11 @@ void QMakeParser::putLineMarker(ushort *&tokPtr)
 
 void QMakeParser::enterScope(ushort *&tokPtr, bool special, ScopeState state)
 {
+    uchar nest = m_blockstack.top().nest;
     m_blockstack.resize(m_blockstack.size() + 1);
     m_blockstack.top().special = special;
     m_blockstack.top().start = tokPtr;
+    m_blockstack.top().nest = nest;
     tokPtr += 2;
     m_state = state;
     m_canElse = false;
@@ -978,6 +986,7 @@ void QMakeParser::finalizeCall(ushort *&tokPtr, ushort *uc, ushort *ptr, int arg
             m_tmp.setRawData((QChar *)uc + 4, nlen);
             const QString *defName;
             ushort defType;
+            uchar nest;
             if (m_tmp == statics.strfor) {
                 if (m_invert || m_operator == OrOperator) {
                     // '|' could actually work reasonably, but qmake does nonsense here.
@@ -1000,6 +1009,7 @@ void QMakeParser::finalizeCall(ushort *&tokPtr, ushort *uc, ushort *ptr, int arg
                       didFor:
                         putTok(tokPtr, TokValueTerminator);
                         enterScope(tokPtr, true, StCtrl);
+                        m_blockstack.top().nest |= NestLoop;
                         return;
                     } else if (*uc == TokArgSeparator && argc == 2) {
                         // for(var, something)
@@ -1046,11 +1056,48 @@ void QMakeParser::finalizeCall(ushort *&tokPtr, ushort *uc, ushort *ptr, int arg
                         putTok(tokPtr, defType);
                         putHashStr(tokPtr, uce + 2, nlen);
                         enterScope(tokPtr, true, StCtrl);
+                        m_blockstack.top().nest = NestFunction;
                         return;
                     }
                 }
                 parseError(fL1S("%1(function) requires one literal argument.").arg(*defName));
                 return;
+            } else if (m_tmp == statics.strreturn) {
+                if (argc > 1) {
+                    parseError(fL1S("return() requires zero or one argument."));
+                    bogusTest(tokPtr);
+                    return;
+                }
+                defType = TokReturn;
+                nest = NestFunction;
+                goto ctrlstm2;
+            } else if (m_tmp == statics.strnext) {
+                defType = TokNext;
+                goto ctrlstm;
+            } else if (m_tmp == statics.strbreak) {
+                defType = TokBreak;
+              ctrlstm:
+                if (*uce != TokFuncTerminator) {
+                    parseError(fL1S("%1() requires zero arguments.").arg(m_tmp));
+                    bogusTest(tokPtr);
+                    return;
+                }
+                nest = NestLoop;
+              ctrlstm2:
+                if (m_invert) {
+                    parseError(fL1S("Unexpected NOT operator in front of %1().").arg(m_tmp));
+                    bogusTest(tokPtr);
+                    return;
+                }
+                if (!(m_blockstack.top().nest & nest)) {
+                    parseError(fL1S("Unexpected %1().").arg(m_tmp));
+                    bogusTest(tokPtr);
+                    return;
+                }
+                finalizeTest(tokPtr);
+                putBlock(tokPtr, uce, ptr - uce - 1); // Only for TokReturn
+                putTok(tokPtr, defType);
+                return;
             } else if (m_tmp == statics.stroption) {
                 if (m_state != StNew || m_blockstack.top().braceLevel || m_blockstack.size() > 1
                         || m_invert || m_operator != NoOperator) {

src/shared/proparser/qmakeparser.h

@@ -83,13 +83,20 @@ public:
                             SubGrammar grammar = FullGrammar);
 
 private:
+    enum ScopeNesting {
+        NestNone = 0,
+        NestLoop = 1,
+        NestFunction = 2
+    };
+
     struct BlockScope {
-        BlockScope() : start(0), braceLevel(0), special(false), inBranch(false) {}
+        BlockScope() : start(0), braceLevel(0), special(false), inBranch(false), nest(NestNone) {}
         BlockScope(const BlockScope &other) { *this = other; }
         ushort *start; // Where this block started; store length here
         int braceLevel; // Nesting of braces in scope
         bool special; // Single-line conditionals inside loops, etc. cannot have else branches
         bool inBranch; // The 'else' branch of the previous TokBranch is still open
+        uchar nest; // Into what control structures we are nested
     };
 
     enum ScopeState {