CallExp must interpret arguments before function/variable 86/17286/6
Cedric Delamarre [Tue, 6 Oct 2015 14:32:55 +0000 (16:32 +0200)]
// error must be undefined variable b
a(b)

test_run ast bug_14135

Change-Id: Ibcf4f6c59faf1db3f1625e353e216e65690be794

scilab/CHANGES_6.0.X
scilab/modules/ast/src/cpp/ast/run_CallExp.hpp
scilab/modules/ast/tests/nonreg_tests/bug_14135.dia.ref [new file with mode: 0644]
scilab/modules/ast/tests/nonreg_tests/bug_14135.tst [new file with mode: 0644]

index e543915..dc0ca63 100644 (file)
@@ -87,6 +87,8 @@ Scilab Bug Fixes
 
 * Bug #14113 fixed - Scilab 6 did not detect infinite loop.
 
+* Bug #14135 fixed - crash when running "Graphics -> Matplot -> Java Image" demonstration.
+
 * Bug #14141 fixed - recursive insertion: gcf().attribute=value => "Wrong insertion : function or macro are not expected".
 
 * Bug #14144 fixed - Scilab crashed with int64(2^63).
index 996315f..4818750 100644 (file)
@@ -16,291 +16,327 @@ namespace ast {
 template<class T>
 void RunVisitorT<T>::visitprivate(const CallExp &e)
 {
-    e.getName().accept(*this);
-    types::InternalType* pIT = getResult();
-
-    if (pIT != NULL)
-    {
-        //function call
-        types::typed_list out;
-        types::typed_list in;
-        types::optional_list opt;
-
-        int iRetCount = getExpectedSize();
-        int iSaveExpectedSize = iRetCount;
+    types::typed_list outTmp;
+    types::typed_list inTmp;
+    std::vector<std::wstring> vectOptName;
+    std::vector<int> vectNbResult;
 
-        // manage case [a,b]=foo() where foo is defined as a=foo()
-        if (pIT->getInvokeNbOut() != -1 && pIT->getInvokeNbOut() < iRetCount)
-        {
-            clearResult();
-            std::wostringstream os;
-            os << _W("Wrong number of output arguments.\n") << std::endl;
-            throw ast::InternalError(os.str(), 999, e.getLocation());
-        }
+    int iRetCount = getExpectedSize();
+    int iSaveExpectedSize = iRetCount;
 
-        //get function arguments
-        exps_t args = e.getArgs();
-        try
+    //get function arguments
+    exps_t args = e.getArgs();
+    try
+    {
+        for (auto& arg : args)
         {
-            for (auto arg : args)
+            if (arg->isAssignExp())
             {
-                if (arg->isAssignExp())
+                AssignExp* pAssign = static_cast<AssignExp*>(arg);
+                //optional parameter
+                Exp* pL = &pAssign->getLeftExp();
+                if (!pL->isSimpleVar())
                 {
-                    AssignExp* pAssign = static_cast<AssignExp*>(arg);
-                    //optional parameter
-                    Exp* pL = &pAssign->getLeftExp();
-                    if (!pL->isSimpleVar())
-                    {
-                        std::wostringstream os;
-                        os << _W("left side of optional parameter must be a variable") << std::endl;
-                        throw ast::InternalError(os.str(), 999, e.getLocation());
-                    }
+                    std::wostringstream os;
+                    os << _W("left side of optional parameter must be a variable") << std::endl;
+                    throw ast::InternalError(os.str(), 999, e.getLocation());
+                }
 
-                    SimpleVar* pVar = pL->getAs<SimpleVar>();
-                    Exp* pR = &pAssign->getRightExp();
-                    pR->accept(*this);
-                    types::InternalType* pITR = getResult();
-                    // IncreaseRef to protect opt argument of scope_end delete
-                    // It will be deleted by clear_opt
-                    pITR->IncreaseRef();
+                SimpleVar* pVar = pL->getAs<SimpleVar>();
+                Exp* pR = &pAssign->getRightExp();
+                pR->accept(*this);
+                types::InternalType* pITR = getResult();
+                // IncreaseRef to protect opt argument of scope_end delete
+                // It will be deleted by clear_opt
+                pITR->IncreaseRef();
 
-                    if (pIT->hasInvokeOption())
-                    {
-                        opt.push_back(std::pair<std::wstring, types::InternalType*>(pVar->getSymbol().getName(), pITR));
-                        //in case of macro/macrofile, we have to shift input param
-                        //so add NULL item in in list to keep initial order
-                        if (pIT->isMacro() || pIT->isMacroFile())
-                        {
-                            in.push_back(NULL);
-                        }
-                    }
-                    else
-                    {
-                        in.push_back(pITR);
-                    }
+                vectOptName.push_back(pVar->getSymbol().getName());
+                inTmp.push_back(pITR);
+                vectNbResult.push_back(1);
 
-                    clearResult();
-                    continue;
-                }
+                clearResult();
+                continue;
+            }
 
-                int iSize = getExpectedSize();
-                setExpectedSize(-1);
-                arg->accept(*this);
-                setExpectedSize(iSize);
+            int iSize = getExpectedSize();
+            setExpectedSize(-1);
+            arg->accept(*this);
+            setExpectedSize(iSize);
 
-                if (getResult() == NULL)
-                {
-                    //special case for empty extraction of list ( list()(:) )
-                    continue;
-                }
+            if (getResult() == NULL)
+            {
+                //special case for empty extraction of list ( list()(:) )
+                vectNbResult.push_back(0);
+                continue;
+            }
 
-                //extract implicit list for call()
-                if (pIT->isCallable() || pIT->isUserType())
+            if (isSingleResult())
+            {
+                inTmp.push_back(getResult());
+                getResult()->IncreaseRef();
+            }
+            else
+            {
+                for (int i = 0; i < getResultSize(); i++)
                 {
-                    types::InternalType * pITArg = getResult();
-                    if (pITArg->isImplicitList())
-                    {
-                        types::ImplicitList* pIL = pITArg->getAs<types::ImplicitList>();
-                        if (pIL->isComputable())
-                        {
-                            setResult(pIL->extractFullMatrix());
-                            pITArg->killMe();
-                        }
-                    }
+                    types::InternalType * pITArg = getResult(i);
+                    pITArg->IncreaseRef();
+                    inTmp.push_back(pITArg);
                 }
+            }
 
-                if (isSingleResult())
+            vectNbResult.push_back(getResultSize());
+            clearResult();
+        }
+    }
+    catch (const InternalError& ie)
+    {
+        clearResult();
+        cleanIn(inTmp, outTmp);
+        throw ie;
+    }
+
+    // get function/variable
+    e.getName().accept(*this);
+    types::InternalType* pIT = getResult();
+
+    types::typed_list out;
+    types::typed_list in;
+    types::optional_list opt;
+
+    // manage case [a,b]=foo() where foo is defined as a=foo()
+    if (pIT->getInvokeNbOut() != -1 && pIT->getInvokeNbOut() < iRetCount)
+    {
+        clearResult();
+        std::wostringstream os;
+        os << _W("Wrong number of output arguments.\n") << std::endl;
+        throw ast::InternalError(os.str(), 999, e.getLocation());
+    }
+
+    // manage input according the function/variable
+    int iLoop = -1;
+    int iterIn = 0;
+    int iterOptName = 0;
+    for (auto& arg : args)
+    {
+        iLoop++;
+
+        //special case for empty extraction of list ( list()(:) )
+        if (vectNbResult[iLoop] == 0)
+        {
+            continue;
+        }
+
+        // management of optional input
+        if (arg->isAssignExp())
+        {
+            if (pIT->hasInvokeOption())
+            {
+                opt.emplace_back(vectOptName[iterOptName++], inTmp[iterIn++]);
+
+                //in case of macro/macrofile, we have to shift input param
+                //so add NULL item in in list to keep initial order
+                if (pIT->isMacro() || pIT->isMacroFile())
                 {
-                    in.push_back(getResult());
-                    getResult()->IncreaseRef();
-                    clearResult();
+                    in.push_back(NULL);
                 }
-                else
-                {
-                    for (int i = 0 ; i < getResultSize() ; i++)
-                    {
-                        types::InternalType * pITArg = getResult(i);
-                        pITArg->IncreaseRef();
-                        in.push_back(pITArg);
-                    }
+            }
+            else
+            {
+                in.push_back(inTmp[iterIn++]);
+            }
+
+            continue;
+        }
 
-                    clearResult();
+        //extract implicit list for call()
+        if (pIT->isCallable() || pIT->isUserType())
+        {
+            if (inTmp[iterIn]->isImplicitList())
+            {
+                types::ImplicitList* pIL = inTmp[iterIn]->getAs<types::ImplicitList>();
+                if (pIL->isComputable())
+                {
+                    types::InternalType* pITExtract = pIL->extractFullMatrix();
+                    pITExtract->IncreaseRef();
+                    in.push_back(pITExtract);
+                    inTmp[iterIn++]->killMe();
+                    continue;
                 }
             }
         }
-        catch (const InternalError& ie)
+
+        // default case
+        for(int i = 0; i < vectNbResult[iLoop]; i++, iterIn++)
         {
-            clearResult();
-            cleanOpt(opt);
-            cleanIn(in, out);
-            throw ie;
+            in.push_back(inTmp[iterIn]);
         }
+    }
 
-        try
+    try
+    {
+        // Extraction with a List in input argument.
+        // This extraction must be a recursive extract.
+        int iLoopSize = 1;
+        types::List* pListArg = NULL;
+        if (pIT->isCallable() == false && in.size() == 1 && in[0]->isList())
         {
-            // Extraction with a List in input argument.
-            // This extraction must be a recursive extract.
-            int iLoopSize = 1;
-            types::List* pListArg = NULL;
-            if (pIT->isCallable() == false && in.size() == 1 && in[0]->isList())
-            {
-                pListArg = in[0]->getAs<types::List>();
-                iLoopSize = pListArg->getSize();
-                cleanOpt(opt);
-            }
+            pListArg = in[0]->getAs<types::List>();
+            iLoopSize = pListArg->getSize();
+            cleanOpt(opt);
+        }
 
-            setExpectedSize(iSaveExpectedSize);
-            iRetCount = std::max(1, iRetCount);
+        setExpectedSize(iSaveExpectedSize);
+        iRetCount = std::max(1, iRetCount);
 
-            for (int i = 0; i < iLoopSize; i++)
+        for (int i = 0; i < iLoopSize; i++)
+        {
+            if (pListArg)
             {
-                if (pListArg)
-                {
-                    in[0] = pListArg->get(i);
+                in[0] = pListArg->get(i);
 
-                    if (in[0]->isList())
+                if (in[0]->isList())
+                {
+                    if (pIT->isCallable())
                     {
-                        if (pIT->isCallable())
+                        // list used like "varargin"
+                        types::List* pLFuncArgs = in[0]->getAs<types::List>();
+                        types::typed_list input;
+                        for (int j = 0; j < pLFuncArgs->getSize(); j++)
                         {
-                            // list used like "varargin"
-                            types::List* pLFuncArgs = in[0]->getAs<types::List>();
-                            types::typed_list input;
-                            for (int j = 0; j < pLFuncArgs->getSize(); j++)
-                            {
-                                input.push_back(pLFuncArgs->get(j));
-                                input.back()->IncreaseRef();
-                            }
-
-                            in = input;
+                            input.push_back(pLFuncArgs->get(j));
+                            input.back()->IncreaseRef();
                         }
-                        else
-                        {
-                            pListArg->DecreaseRef();
-                            pListArg->killMe();
 
-                            std::wostringstream os;
-                            os << _W("Invalid index.\n");
-                            throw ast::InternalError(os.str(), 999, e.getFirstLocation());
-                        }
+                        in = input;
                     }
                     else
                     {
-                        in[0]->IncreaseRef();
+                        pListArg->DecreaseRef();
+                        pListArg->killMe();
+
+                        std::wostringstream os;
+                        os << _W("Invalid index.\n");
+                        throw ast::InternalError(os.str(), 999, e.getFirstLocation());
                     }
                 }
+                else
+                {
+                    in[0]->IncreaseRef();
+                }
+            }
 
-                bool ret = false;
-                if(pIT->isInvokable() == false)
+            bool ret = false;
+            if (pIT->isInvokable() == false)
+            {
+                // call overload
+                ret = Overload::call(L"%" + pIT->getShortTypeStr() + L"_e", in, iRetCount, out, this);
+            }
+            else
+            {
+                ret = pIT->invoke(in, opt, iRetCount, out, e);
+                if (ret == false && pIT->isUserType())
                 {
                     // call overload
                     ret = Overload::call(L"%" + pIT->getShortTypeStr() + L"_e", in, iRetCount, out, this);
                 }
-                else
+            }
+
+            if (ret)
+            {
+                if (iSaveExpectedSize != -1 && iSaveExpectedSize > out.size())
                 {
-                    ret = pIT->invoke(in, opt, iRetCount, out, e);
-                    if(ret == false && pIT->isUserType())
+                    char szError[bsiz];
+                    if (pIT->isCallable())
                     {
-                        // call overload
-                        ret = Overload::call(L"%" + pIT->getShortTypeStr() + L"_e", in, iRetCount, out, this);
+                        char* strFName = wide_string_to_UTF8(pIT->getAs<types::Callable>()->getName().c_str());
+                        os_sprintf(szError, _("%s: Wrong number of output argument(s): %d expected.\n"), strFName, out.size());
+                        FREE(strFName);
                     }
-                }
-
-                if (ret)
-                {
-                    if (iSaveExpectedSize != -1 && iSaveExpectedSize > out.size())
+                    else
                     {
-                        char szError[bsiz];
-                        if(pIT->isCallable())
-                        {
-                            char* strFName = wide_string_to_UTF8(pIT->getAs<types::Callable>()->getName().c_str());
-                            os_sprintf(szError, _("%s: Wrong number of output argument(s): %d expected.\n"), strFName, out.size());
-                            FREE(strFName);
-                        }
-                        else
-                        {
-                            os_sprintf(szError, _("%s: Wrong number of output argument(s): %d expected.\n"), "extract", out.size());
-                        }
-
-                        wchar_t* wError = to_wide_string(szError);
-                        std::wstring err(wError);
-                        FREE(wError);
-                        throw InternalError(err, 999, e.getLocation());
+                        os_sprintf(szError, _("%s: Wrong number of output argument(s): %d expected.\n"), "extract", out.size());
                     }
 
-                    setExpectedSize(iSaveExpectedSize);
-                    setResult(out);
-                    cleanIn(in, out);
-                    cleanOpt(opt);
-
-                    // In case a.b(), getResult contain pIT ("b").
-                    // If out == pIT, do not delete it.
-                    if (getResult() != pIT)
-                    {
-                        // protect element of out in case where
-                        // out contain elements of pIT
-                        for (int i = 0; i < out.size(); i++)
-                        {
-                            out[i]->IncreaseRef();
-                        }
+                    wchar_t* wError = to_wide_string(szError);
+                    std::wstring err(wError);
+                    FREE(wError);
+                    throw InternalError(err, 999, e.getLocation());
+                }
 
-                        pIT->killMe();
+                setExpectedSize(iSaveExpectedSize);
+                setResult(out);
+                cleanIn(in, out);
+                cleanOpt(opt);
 
-                        // unprotect
-                        for (int i = 0; i < out.size(); i++)
-                        {
-                            out[i]->DecreaseRef();
-                        }
+                // In case a.b(), getResult contain pIT ("b").
+                // If out == pIT, do not delete it.
+                if (getResult() != pIT)
+                {
+                    // protect element of out in case where
+                    // out contain elements of pIT
+                    for (int i = 0; i < out.size(); i++)
+                    {
+                        out[i]->IncreaseRef();
                     }
 
-                    if (pListArg && i + 1 != iLoopSize)
+                    pIT->killMe();
+
+                    // unprotect
+                    for (int i = 0; i < out.size(); i++)
                     {
-                        pIT = out[0];
-                        out.clear();
-                        setResult(NULL);
+                        out[i]->DecreaseRef();
                     }
                 }
-                else
+
+                if (pListArg && i + 1 != iLoopSize)
                 {
-                    std::wostringstream os;
-                    os << _W("Invalid index.\n");
-                    throw ast::InternalError(os.str(), 999, e.getFirstLocation());
+                    pIT = out[0];
+                    out.clear();
+                    setResult(NULL);
                 }
             }
-
-            if (pListArg)
+            else
             {
-                pListArg->DecreaseRef();
-                pListArg->killMe();
+                std::wostringstream os;
+                os << _W("Invalid index.\n");
+                throw ast::InternalError(os.str(), 999, e.getFirstLocation());
             }
         }
-        catch (InternalAbort & ia)
-        {
-            setExpectedSize(iSaveExpectedSize);
-            if(pIT != getResult())
-            {
-                pIT->killMe();
-            }
-
-            clearResult();
-            cleanInOut(in, out);
-            cleanOpt(opt);
 
-            throw ia;
+        if (pListArg)
+        {
+            pListArg->DecreaseRef();
+            pListArg->killMe();
         }
-        catch (const InternalError& ie)
+    }
+    catch (InternalAbort & ia)
+    {
+        setExpectedSize(iSaveExpectedSize);
+        if (pIT != getResult())
         {
-            setExpectedSize(iSaveExpectedSize);
-            if(pIT != getResult())
-            {
-                pIT->killMe();
-            }
+            pIT->killMe();
+        }
 
-            clearResult();
-            cleanInOut(in, out);
-            cleanOpt(opt);
+        clearResult();
+        cleanInOut(in, out);
+        cleanOpt(opt);
 
-            throw ie;
+        throw ia;
+    }
+    catch (const InternalError& ie)
+    {
+        setExpectedSize(iSaveExpectedSize);
+        if (pIT != getResult())
+        {
+            pIT->killMe();
         }
+
+        clearResult();
+        cleanInOut(in, out);
+        cleanOpt(opt);
+
+        throw ie;
     }
 }
 
diff --git a/scilab/modules/ast/tests/nonreg_tests/bug_14135.dia.ref b/scilab/modules/ast/tests/nonreg_tests/bug_14135.dia.ref
new file mode 100644 (file)
index 0000000..f895c09
--- /dev/null
@@ -0,0 +1,18 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2015 - Scilab Enterprises - Cedric Delamarre
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+// <-- CLI SHELL MODE -->
+//
+// <-- Non-regression test for bug 14135 -->
+//
+// <-- Bugzilla URL -->
+// http://bugzilla.scilab.org/14135
+//
+// <-- Short Description -->
+// crash when running "Graphics -> Matplot -> Java Image" demonstration.
+// argument of a "function call"/"extraction" must be treated before the function/variable.
+expected = msprintf(gettext("Undefined variable: %s\n"), "b");
+assert_checkerror("a(b)",expected);
diff --git a/scilab/modules/ast/tests/nonreg_tests/bug_14135.tst b/scilab/modules/ast/tests/nonreg_tests/bug_14135.tst
new file mode 100644 (file)
index 0000000..bdf4e34
--- /dev/null
@@ -0,0 +1,19 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2015 - Scilab Enterprises - Cedric Delamarre
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+// <-- CLI SHELL MODE -->
+//
+// <-- Non-regression test for bug 14135 -->
+//
+// <-- Bugzilla URL -->
+// http://bugzilla.scilab.org/14135
+//
+// <-- Short Description -->
+// crash when running "Graphics -> Matplot -> Java Image" demonstration.
+// argument of a "function call"/"extraction" must be treated before the function/variable.
+
+expected = msprintf(gettext("Undefined variable: %s\n"), "b");
+assert_checkerror("a(b)",expected);