basic/qa/basic_coverage/test_iif_method.bas |   27 ++++++++++++++++++++-------
 basic/source/runtime/runtime.cxx            |   18 +++++++++++++++++-
 2 files changed, 37 insertions(+), 8 deletions(-)

New commits:
commit a7e4d70072314957fd3484251f3775099097cd2a
Author:     Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Sun Jun 29 03:27:50 2025 +0500
Commit:     Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Tue Jul 1 09:08:12 2025 +0200

    tdf#149151: disambiguate method parameters and array indexes
    
    SbiRuntime::FindElement ends with a call to CheckArray(). The latter
    takes a variable, checks if that is an array and if it has parameters;
    and if both are true, tries to obtain an element of the array defined
    by the parameters as array indexes (using SbxDimArray::Get).
    
    The problem was, that the variable may actually be array; it may have
    parameters; but this array can be the result of a function call; the
    variable may be SbxMethod; and the parameters may be for the function,
    not array indexes.
    
    Specifically, the call like
    
      Iif(True, Array("A","B"), Array("B","A"))
    
    searched for an entity named Iif; found that it was a RTL method;
    prepared the arguments for it (consisting of four elements: 0 - the
    method itself; 1 - boolean True; 2 - array ["A", "B"]; 3 - array
    ["B", "A"]); the function executed, putting the correct value to the
    params[0], which was the method itself (so its value became an array);
    and finally, the SbxMethod variable with the array value got passed
    to CheckArray. The latter found out that it's an array, and that it
    has parameters (those four) - and tried to get the element, which
    index was defined by the parameters.
    
    Here comes the specifics of LibreOffice Basic's argument handling.
    SbxDimArray::Get only cares about first arguments expected by the
    array dimensions; it ignores the rest. Since the array was single-
    dimensional, it only considered parameter 1 (parameter 0 is, by
    convention, reserved for return value). That was a boolean. When it
    was False, its GetInteger returned 0; and finally, CheckArray took
    element 0 from the array, returning a scalar value, as if it was
    
      Iif(False, Array("A","B"), Array("B","A"))(0)
    
    But when the boolean was True, another Basic specific took place:
    its numerical value is -1; and trying to get element -1 from the
    array resulted in "Index out of defined range".
    
    This change disambiguates between the cases of array index, and
    function parameters, checking if the variable itself, and its 0th
    parameter are both SbxMethods. Then it doesn't try to treat the
    parameters as indexes into the array.
    
    Change-Id: I597f94a7e688c8bba12d87a1bd5871491c25cac0
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187145
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
    (cherry picked from commit 99247bfc4e42652baddf606f90e31fb4d22006ab)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/187165
    Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>

diff --git a/basic/qa/basic_coverage/test_iif_method.bas 
b/basic/qa/basic_coverage/test_iif_method.bas
index b6003d052077..060aef188337 100644
--- a/basic/qa/basic_coverage/test_iif_method.bas
+++ b/basic/qa/basic_coverage/test_iif_method.bas
@@ -8,11 +8,24 @@
 
 Option Explicit
 
-Function doUnitTest as String
-    ' IIF
-    If ( IIF(True, 10, 12) <> 10 ) Then
-        doUnitTest = "FAIL"
-    Else
-        doUnitTest = "OK"
-    End If
+Function doUnitTest() As String
+    TestUtil.TestInit
+    verify_testIif
+    doUnitTest = TestUtil.GetResult()
 End Function
+
+Sub verify_testIif
+    On Error GoTo errorHandler
+
+    TestUtil.AssertEqual(IIF(True, 10, 12), 10, "IIF(True, 10, 12)")
+
+    ' tdf#149151
+    ' Without the fix, this would fail with "hit error handler - 9: Index out 
of defined range"
+    TestUtil.Assert(IsArray(Iif(True, Array("A","B"), Array("B","A"))), 
"IsArray(Iif(True, Array(""A"",""B""), Array(""B"",""A"")))")
+    TestUtil.AssertEqualStrict(Iif(True, Array("A","B"), Array("B","A"))(0), 
"A", "Iif(True, Array(""A"",""B""), Array(""B"",""A""))(0)")
+    TestUtil.AssertEqualStrict(Iif(True, Array("A","B"), Array("B","A"))(1), 
"B", "Iif(True, Array(""A"",""B""), Array(""B"",""A""))(1)")
+
+    Exit Sub
+errorHandler:
+    TestUtil.ReportErrorHandler("verify_testIif", Err, Error$, Erl)
+End Sub
diff --git a/basic/source/runtime/runtime.cxx b/basic/source/runtime/runtime.cxx
index 0b66d81fde8e..8a4946822507 100644
--- a/basic/source/runtime/runtime.cxx
+++ b/basic/source/runtime/runtime.cxx
@@ -3953,7 +3953,23 @@ SbxVariable* SbiRuntime::CheckArray( SbxVariable* pElem )
             // parameters may be missing, if an array is
             // passed as an argument
             if( pPar )
-                pElem = pDimArray->Get( pPar );
+            {
+                bool parIsArrayIndex = true;
+                if (dynamic_cast<const SbxMethod*>(pElem))
+                {
+                    // If this was a method, then there are two possibilities:
+                    // 1. pPar is this method's parameters.
+                    // 2. pPar is the indexes into the array returned from the 
method.
+                    // To disambiguate, check the 0th element of pPar.
+                    if (dynamic_cast<const SbxMethod*>(pPar->Get(0)))
+                    {
+                        // pPar was the parameters to the method, not indexes 
into the array
+                        parIsArrayIndex = false;
+                    }
+                }
+                if (parIsArrayIndex)
+                    pElem = pDimArray->Get(pPar);
+            }
         }
         else
         {

Reply via email to