From 4e275fac3626fd00c02f13a0205fba17aa45fab9 Mon Sep 17 00:00:00 2001
From: Steve Bennett <steveb@workware.net.au>
Date: Tue, 5 Nov 2019 18:16:27 +1000
Subject: [PATCH] expr-sugar: $() should return non-error codes

If an expression returns (e.g.), break, continue or exit, that
return code should be propagated, the same it is for [expr {}]

Signed-off-by: Steve Bennett <steveb@workware.net.au>
---
 jim.c                | 46 ++++++++++++++++++++++------------------------
 tests/exprsugar.test |  5 +++++
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/jim.c b/jim.c
index eed6e70..ab67141 100644
--- a/jim.c
+++ b/jim.c
@@ -4922,14 +4922,6 @@ static Jim_Obj *JimExpandDictSugar(Jim_Interp *interp, Jim_Obj *objPtr)
     return resObjPtr;
 }
 
-static Jim_Obj *JimExpandExprSugar(Jim_Interp *interp, Jim_Obj *objPtr)
-{
-    if (Jim_EvalExpression(interp, objPtr) == JIM_OK) {
-        return Jim_GetResult(interp);
-    }
-    return NULL;
-}
-
 /* -----------------------------------------------------------------------------
  * CallFrame
  * ---------------------------------------------------------------------------*/
@@ -10248,6 +10240,7 @@ static void JimAddErrorToStack(Jim_Interp *interp, ScriptObj *script)
 static int JimSubstOneToken(Jim_Interp *interp, const ScriptToken *token, Jim_Obj **objPtrPtr)
 {
     Jim_Obj *objPtr;
+    int ret = JIM_ERR;
 
     switch (token->type) {
         case JIM_TT_STR:
@@ -10261,22 +10254,21 @@ static int JimSubstOneToken(Jim_Interp *interp, const ScriptToken *token, Jim_Ob
             objPtr = JimExpandDictSugar(interp, token->objPtr);
             break;
         case JIM_TT_EXPRSUGAR:
-            objPtr = JimExpandExprSugar(interp, token->objPtr);
+            ret = Jim_EvalExpression(interp, token->objPtr);
+            if (ret == JIM_OK) {
+                objPtr = Jim_GetResult(interp);
+            }
+            else {
+                objPtr = NULL;
+            }
             break;
         case JIM_TT_CMD:
-            switch (Jim_EvalObj(interp, token->objPtr)) {
-                case JIM_OK:
-                case JIM_RETURN:
-                    objPtr = interp->result;
-                    break;
-                case JIM_BREAK:
-                    /* Stop substituting */
-                    return JIM_BREAK;
-                case JIM_CONTINUE:
-                    /* just skip this one */
-                    return JIM_CONTINUE;
-                default:
-                    return JIM_ERR;
+            ret = Jim_EvalObj(interp, token->objPtr);
+            if (ret == JIM_OK || ret == JIM_RETURN) {
+                objPtr = interp->result;
+            } else {
+                /* includes JIM_BREAK, JIM_CONTINUE */
+                objPtr = NULL;
             }
             break;
         default:
@@ -10289,7 +10281,7 @@ static int JimSubstOneToken(Jim_Interp *interp, const ScriptToken *token, Jim_Ob
         *objPtrPtr = objPtr;
         return JIM_OK;
     }
-    return JIM_ERR;
+    return ret;
 }
 
 /* Interpolate the given tokens into a unique Jim_Obj returned by reference
@@ -10537,7 +10529,13 @@ int Jim_EvalObj(Jim_Interp *interp, Jim_Obj *scriptObjPtr)
                         wordObjPtr = Jim_GetVariable(interp, token[i].objPtr, JIM_ERRMSG);
                         break;
                     case JIM_TT_EXPRSUGAR:
-                        wordObjPtr = JimExpandExprSugar(interp, token[i].objPtr);
+                        retcode = Jim_EvalExpression(interp, token[i].objPtr);
+                        if (retcode == JIM_OK) {
+                            wordObjPtr = Jim_GetResult(interp);
+                        }
+                        else {
+                            wordObjPtr = NULL;
+                        }
                         break;
                     case JIM_TT_DICTSUGAR:
                         wordObjPtr = JimExpandDictSugar(interp, token[i].objPtr);
diff --git a/tests/exprsugar.test b/tests/exprsugar.test
index 9579203..943945a 100644
--- a/tests/exprsugar.test
+++ b/tests/exprsugar.test
@@ -48,5 +48,10 @@ test exprsugar-1.11 {Simple operations} {
 test exprsugar-1.12 {Simple operations} {
 	set x $((2 + 4))
 } 6
+# This necessary to ensure that things like exit will pass through expr-sugar
+test exprsugar-1.13 {Non-error return inside expr-sugar} -body {
+	proc a {} { break }
+	set x $([a])
+} -returnCodes break
 
 testreport
-- 
2.11.4.GIT