bugGNU Octave - Bugs: bug #48656, overridden class subsref method...

 
 

bug #48656: overridden class subsref method may get wrong value for nargout

Submitted by:  Mike Miller <mtmiller>
Submitted on:  Thu 28 Jul 2016 09:44:07 PM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: Wont FixAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Wed 19 Jul 2017 01:00:50 AM UTC, comment #5:

Please reopen this as a bug.

Compare the below octave output with the matlab output.

You can see that, unless it's 0 or 1, matlab does in fact take into account the number of arguments requested.

(PS. I also note bug #48693 is related and open; the behaviour there is also more compatible with matlab's behaviour here, though not identical since the 0 and 1 cases differ.)

Tasos Papastylianou <tpapastylianou>
Mon 01 Aug 2016 05:12:11 PM UTC, comment #4:

I suppose dot indexing of classes also returning more than one value is intended to match struct array indexing. So I guess I'll close this as working.

Mike Miller <mtmiller>
Project Administrator
Fri 29 Jul 2016 08:28:46 PM UTC, comment #3:

I think these two help pages describe the relevant behavior:

http://www.mathworks.com/help/matlab/matlab_oop/number-of-arguments-in-indexing-methods.html
http://www.mathworks.com/help/matlab/matlab_oop/overloading-numel-subsref-and-subsasgn.html

So yes, the number of indices is supposed to set what the value of nargout is, not the normal number of output arguments.

But I'm still curious why or whether this should apply to the first index type being ".".

Mike Miller <mtmiller>
Project Administrator
Thu 28 Jul 2016 10:06:33 PM UTC, comment #2:

The attached diff gives me the behavior that I want, but again, unclear to me whether this is a safe or desired change.

(file #38049)

Mike Miller <mtmiller>
Project Administrator
Thu 28 Jul 2016 09:49:02 PM UTC, comment #1:

I can take a guess that nargout is changed for brace indexing to make it look like cell array indexing, which returns a cs-list of values rather than a single array of values. But why for dot indexing? And is there a syntactical way to do dot indexing with more than one index value?

Mike Miller <mtmiller>
Project Administrator
Thu 28 Jul 2016 09:44:07 PM UTC, original submission:

When a user class overrides the subsref method to implement dot indexing, the wrong value of nargout may be passed in to subsref, incorrectly representing the number of items to be indexed rather than the number of outputs that the caller actually wanted.

There may be a Matlab compatible reason for this, judging by the comments in the code that implements this logic, but I don't understand it and I don't have access to Matlab to test.

The block of code starting here is what adjusts the value of nargout depending on what indices were passed in to the object: http://hg.savannah.gnu.org/hgweb/octave/file/829e0aafebdc/libinterp/octave-value/ov-class.cc#l437

This is largely due to these commits:

http://hg.savannah.gnu.org/hgweb/octave/rev/a76f391a3d02
http://hg.savannah.gnu.org/hgweb/octave/rev/318e0cdd31bd

Can someone decipher whether this adjustment is true and necessary? Can someone with access to Matlab test what nargout is inside of the subsref function for various indexing examples?

Here are some tests showing the problem in Octave:

Mike Miller <mtmiller>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #38049:  bug-48656.diff added by mtmiller (1KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by tpapastylianou (Posted a comment)
  • -unavailable- added by mtmiller (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 01 Aug 2016 05:12:11 PM UTCmtmillerStatusNeed Info=>Wont Fix
      Open/ClosedOpen=>Closed
    Thu 28 Jul 2016 10:06:33 PM UTCmtmillerAttached File-=>Added bug-48656.diff, #38049
      StatusNone=>Need Info

    Back to the top


    Powered by Savane 3.1-cleanup1