bugGNU Octave - Bugs: bug #63841, nargout is always 0 or 1 for...

 
 

bug #63841: nargout is always 0 or 1 for subsref() in classdef methods

Submitter:  A.R. Burgers <arb>
Submitted:  Sun 26 Feb 2023 10:23:21 PM UTC
 
Category:  Octave Function Severity:  5 - Blocker
Priority:  7 - High Item Group:  Regression
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  9.1.0 (current default)
* Mandatory Fields

Add a New Comment (Rich Markup)
   

( Jump to the original submission )

Mon 27 Feb 2023 04:58:00 PM UTC, comment #10: 

I don't pretend this is brilliant, but this patch seems to fix the case for the two reported bugs.


diff --git a/libinterp/octave-value/ov-classdef.cc b/libinterp/octave-value/ov-classdef.cc
--- a/libinterp/octave-value/ov-classdef.cc
+++ b/libinterp/octave-value/ov-classdef.cc
@@ -102,8 +102,9 @@ octave_classdef::subsref (const std::str
             {
               // Set up a proper nargout for the subsref call by calling numel.
               octave_value_list tmp;
-              if (type[0] != '.') tmp = idx.front ();
-              nargout = xnumel (tmp);
+              if (type[0] != '.')
+                tmp = idx.front ();
+              nargout = std::max (static_cast<int> (xnumel (tmp)), nargout);
             }

           retval = meth.execute (args, nargout, true, "subsref");


All it does is use the maximum value for nargout which is either the number of output variables asked for by the interpeter (nargout input parameter to function) or the maximum number of outputs for a varargout expression taken from the index operation.

(file #54400)

Rik <rik5>
Project Administrator
Mon 27 Feb 2023 04:28:03 PM UTC, comment #9: 

Comment #8 (subsref works for (), not for {} or .) seems consistent with the changeset that jwe identified by bisection.  That cset begins with


+          // Attempt to set up a proper value for nargout at least in the
+          // simple case where the cs-list-type expression - i.e., {} or
+          // ().x, is the leading one.
+          bool maybe_cs_list_query = (type[0] == '.' || type[0] == '{'
+                                      || (type.length () > 1 && type[0] == '('
+                                          && type[1] == '.'));
+
+          if (maybe_cs_list_query)
+            {
+              // Set up a proper nargout for the subsref call by calling numel.
+              octave_value_list tmp;
+              if (type[0] != '.') tmp = idx.front ();
+              nargout = xnumel (tmp);
+            }


The comment in the code about the variable "maybe_cs_list_query" implies that this is about calculating the specific cases of "{}" and ".".

Rik <rik5>
Project Administrator
Mon 27 Feb 2023 02:32:26 PM UTC, comment #8: 

for () indexing also octave-9 gets it right, but not for {} and .

class tst_nargout:


classdef tst_nargout
  methods
    function varargout = subsref(obj, s)
      fprintf('nargout=%d, s(1).type = %s\n', nargout, s(1).type);
    end
  end
end


exercising tst_nargout:


ti = tst_nargout;

fprintf('2 outputs requested\n');
try; [a, b] = ti.call_a_method; catch; end
try; [a, b] = ti{1};            catch; end
try; [a, b] = ti(1);            catch; end

fprintf('3 outputs requested\n');
try; [a, b, c] = ti.call_a_method; catch; end
try; [a, b, c] = ti{1};            catch; end
try; [a, b, c] = ti(1);            catch; end


octave-8 output


2 outputs requested
nargout=2, s(1).type = .
nargout=2, s(1).type = {}
nargout=2, s(1).type = ()
3 outputs requested
nargout=3, s(1).type = .
nargout=3, s(1).type = {}
nargout=3, s(1).type = ()


octave-9 output


2 outputs requested
nargout=1, s(1).type = .
nargout=1, s(1).type = {}
nargout=2, s(1).type = ()
3 outputs requested
nargout=1, s(1).type = .
nargout=1, s(1).type = {}
nargout=3, s(1).type = ()


A.R. Burgers <arb>
Mon 27 Feb 2023 05:40:47 AM UTC, comment #7: 

Bisecting points to the following change that was made in an attempt to fix bug #48693:

http://hg.savannah.gnu.org/hgweb/octave/rev/a18897e4c7b5

John W. Eaton <jwe>
Project Administrator
  Spam posted by anonymous
Mon 27 Feb 2023 02:28:12 AM UTC, comment #5: 

I take back my remarks in comment #1: this seems to be specific to subsref().

Rik <rik5>
Project Administrator
Mon 27 Feb 2023 02:14:41 AM UTC, comment #4: 

It is probably tangentially related to bug #48693, as in the fix probably is in the same section of code, but this is slightly different.  nargout is simply the wrong value.

Rik <rik5>
Project Administrator
Mon 27 Feb 2023 02:06:14 AM UTC, comment #3: 
Nicholas Jankowski <nrjank>
Project Member
Mon 27 Feb 2023 01:50:14 AM UTC, comment #2: 

Clearly Octave does not have a sufficient test suite for classdef code because this should have been found much sooner.

Rik <rik5>
Project Administrator
Mon 27 Feb 2023 01:48:22 AM UTC, comment #1: 

Confirmed.  And the problem seems to be general to any method of a classdef object.  I used the following code:


classdef tst_nargout
  methods
    function varargout = subsref (obj, s)
      nout = nargout
    end

    function varargout = myfunc (obj, s)
      nout = nargout
    end

  end
end


Using nargout in an ordinary method then produces the wrong answer.


o = tst_nargout
o =

  tst_nargout object with properties:


octave:2> [a,b,c] = o.myfunc ()
error: element number 1 undefined in return list
nout = 1




(file #54398)

Rik <rik5>
Project Administrator
Sun 26 Feb 2023 10:23:21 PM UTC, original submission:  

consider this minimal class with a subsref method


classdef foo
  methods
    function varargout = subsref(obj, s)
      no = nargout
    end
  end
end


and this use of the foo class, which requests subsref to return 2 outputs:


fi = foo;
[a, b] = fi.call_a_method;


octave-8.0.91 - correctly - returns 2 for no, octave-9.0.0 returns 1

octave-9.0.0 hg-id=c80cf1588ed0

A.R. Burgers <arb>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #54400:  patch.63841 added by rik5 (702B - application/octet-stream)
file #54398:  tst_nargout.m added by rik5 (184B - text/x-matlab)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by arb (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only project members can vote.

     

     

     

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-02-27 rik5 Dependencies- Depends on bugs #48693
    2023-02-27 rik5 Attached File- Added patch.63841, #54400
    2023-02-27 rik5 Summarynargout is always 1 for classdef methods nargout is always 0 or 1 for subsref() in classdef methods
    2023-02-27 rik5 Planned ReleaseNone 9.1.0 (current default)
    2023-02-27 rik5 Attached File- Added tst_nargout.m, #54398
        Severity3 - Normal 5 - Blocker
        Priority5 - Normal 7 - High
        StatusNone Confirmed
        Summarysubsref gets wrong number of varargout elements nargout is always 1 for classdef methods

    Back to the top

    Powered by Savane 3.11