Mercurial > octave

changeset 32414:385e847edfb7

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix nargout -1 for classdefs subsrefs from VM (bug #64775) Support calling subsref with nargout -1 as binary and unary ops make nargout unknown, for classdef that might return cs-lists. * libinterp/parse-tree/pt-bytecode-vm.cc: Handle nargout -1 * libinterp/parse-tree/pt-bytecode-walk.cc: nargout -1 for some . and { subsrefs * libinterp/parse-tree/pt-bytecode-walk.h: New field used to track if nargout -1 could be needed with
author Petter T. <petter.vilhelm@gmail.com>
date Sun, 15 Oct 2023 16:50:47 +0200
parents ee5d0bded6d4
children b747729c6695
files libinterp/parse-tree/pt-bytecode-vm.cc libinterp/parse-tree/pt-bytecode-walk.cc libinterp/parse-tree/pt-bytecode-walk.h
diffstat 3 files changed, 54 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/parse-tree/pt-bytecode-vm.cc	Sat Oct 14 19:13:12 2023 +0200
+++ b/libinterp/parse-tree/pt-bytecode-vm.cc	Sun Oct 15 16:50:47 2023 +0200
@@ -5031,6 +5031,13 @@
 index_struct_call:
   {
     int nargout = arg0;
+    // TODO: Kludge alert. Mirror the behaviour in ov_classdef::subsref
+    // where under certain conditions a magic number nargout of -1 is
+    // expected to  maybe return a cs-list. "-1" in this context 
+    // does not have the same meaning as in the VM, where it means
+    // a varargout with only one return symbol 'varargout'.
+    if (nargout == 255)
+      nargout = -1;
     bool has_slot = *ip++;
     int slot = POP_CODE_USHORT ();
 
@@ -5217,6 +5224,13 @@
     CATCH_EXIT_EXCEPTION
 
     STACK_DESTROY (1);
+  // TODO: Kludge alert. Mirror the behaviour in ov_classdef::subsref
+  // where under certain conditions a magic number nargout of -1 is
+  // expected to  maybe return a cs-list. "-1" in this context 
+  // does not have the same meaning as in the VM, where it means
+  // a varargout with only one return symbol 'varargout'.
+    if (nargout == -1)
+      nargout = 1;
     EXPAND_CSLIST_PUSH_N_OVL_ELEMENTS_TO_STACK (retval, nargout);
   }
   DISPATCH ();
--- a/libinterp/parse-tree/pt-bytecode-walk.cc	Sat Oct 14 19:13:12 2023 +0200
+++ b/libinterp/parse-tree/pt-bytecode-walk.cc	Sun Oct 15 16:50:47 2023 +0200
@@ -1397,6 +1397,7 @@
 bytecode_walker::
 visit_postfix_expression (tree_postfix_expression& expr)
 {
+  m_unknown_nargout++;
   INC_DEPTH();
 
   tree_expression *e = expr.operand ();
@@ -1524,12 +1525,14 @@
   maybe_emit_bind_ans_and_disp (expr);
 
   DEC_DEPTH();
+  m_unknown_nargout--;
 }
 
 void
 bytecode_walker::
 visit_prefix_expression (tree_prefix_expression& expr)
 {
+  m_unknown_nargout++;
   INC_DEPTH();
 
   tree_expression *e = expr.operand ();
@@ -1657,6 +1660,7 @@
   maybe_emit_bind_ans_and_disp (expr);
 
   DEC_DEPTH();
+  m_unknown_nargout--;
 }
 
 void
@@ -1773,6 +1777,7 @@
   // do fused for a matrix, like M'*A etc.
   INC_DEPTH();
   PUSH_NARGOUT (1);
+  m_unknown_nargout++;
 
   tree_expression *op1 = expr.clhs ();
 
@@ -1814,6 +1819,7 @@
 
   POP_NARGOUT ();
   DEC_DEPTH();
+  m_unknown_nargout--;
 }
 
 void
@@ -1822,6 +1828,7 @@
 {
   INC_DEPTH ();
   PUSH_NARGOUT (1);
+  m_unknown_nargout++;
 
   std::vector<int> need_after;
   int fold_slot = -1;
@@ -2100,6 +2107,7 @@
   POP_NARGOUT ();
 
   DEC_DEPTH ();
+  m_unknown_nargout--;
 }
 
 void
@@ -5176,12 +5184,30 @@
 
   arg_name_entry.m_ip_start = CODE_SIZE ();
 
+  // TODO: Kludge alert. Mirror the behaviour in ov_classdef::subsref
+  // where under certain conditions a magic number nargout of -1 is
+  // expected to  maybe return a cs-list. "-1" in this context 
+  // does not have the same meaning as in the VM, where it means
+  // a varargout with only one return symbol 'varargout'.
+  bool m1_magic_nargout = false;
+  if (m_unknown_nargout)
+    {
+      if ((v_types.size () >= 1 && (v_types[0] == '{' || v_types[0] == '.')) ||
+          (v_types.size () >= 2 && (v_types[0] == '(' && v_types[1] == '.')))
+      {
+          m1_magic_nargout = true; 
+      }
+    }
+
   if (first_expression && first_expression->is_identifier ())
     {
       int slot = SLOT (first_expression->name ());
       MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout);
       PUSH_CODE (INSTR::INDEX_STRUCT_CALL);
-      PUSH_CODE (nargout);
+      if (m1_magic_nargout)
+        PUSH_CODE (-1);
+      else
+        PUSH_CODE (nargout);
       PUSH_CODE (1); // has slot
       PUSH_WSLOT (slot); // the slot
     }
@@ -5189,7 +5215,10 @@
     {
       MAYBE_PUSH_ANON_NARGOUT_OPEXT (nargout);
       PUSH_CODE (INSTR::INDEX_STRUCT_CALL);
-      PUSH_CODE (nargout);
+      if (m1_magic_nargout)
+        PUSH_CODE (-1);
+      else
+        PUSH_CODE (nargout);
       PUSH_SLOT (0); // has slot
       PUSH_WSLOT (0); // slot ignored
     }
--- a/libinterp/parse-tree/pt-bytecode-walk.h	Sat Oct 14 19:13:12 2023 +0200
+++ b/libinterp/parse-tree/pt-bytecode-walk.h	Sun Oct 15 16:50:47 2023 +0200
@@ -197,6 +197,15 @@
     // for anonymous functions.
     stack_frame::local_vars_map *m_anon_local_values = nullptr;
 
+    // TODO: Kludge alert. Mirror the behaviour in ov_classdef::subsref
+    // where under certain conditions a magic number nargout of -1 is
+    // expected to  maybe return a cs-list. "-1" in this context 
+    // does not have the same meaning as in the VM, where it means
+    // a varargout with only one return symbol 'varargout'.
+    //
+    // We need to track "unknown nargout" for this.
+    int m_unknown_nargout = 0;
+
     //
     bool m_is_folding = false;
     std::vector<tree*> m_v_trees_to_fold;