bugGNU Octave - Bugs: bug #48693, classdef subsref method is not...

 
 

bug #48693: classdef subsref method is not called with correct nargout value

Submitter:  Mike Miller <mtmiller>
Submitted:  Wed 03 Aug 2016 10:02:56 PM UTC
   
 
Category:  Classdef Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  9.1.0 Planned Release:  9.1.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 14 Oct 2023 01:08:37 PM UTC, comment #11: 

@Fernando: Thank you for the pointer to #63841. I had not known of that before and I've pushed it since it was marked as a high priority blocker. I agree with nrjank: the discussion can be carried on there, leaving this report closed.

Arun Giridhar <arungiridhar>
Group Member
Sat 14 Oct 2023 12:02:27 PM UTC, comment #10: 

Noted. Will take a look over there. At first glance it seems that the patch and fix builds upon this one and there isn't a suggestion of rolling this one back, so will leave this closed and we can track the correction over at bug #63841.

Nicholas Jankowski <nrjank>
Group Member
Sat 14 Oct 2023 09:21:29 AM UTC, comment #9: 

Bear in mind that this fix introduces a regression described in bug #63841, where a fix for the regression has also been proposed.

Fernando <tutissanalio>
Sat 14 Oct 2023 03:33:38 AM UTC, comment #8: 

No, I believe this is complete.  Closing report as fixed.  If something related pops up it can either be reopened or a new report made.

Nicholas Jankowski <nrjank>
Group Member
Fri 13 Oct 2023 07:10:01 PM UTC, comment #7: 

This was pushed in Dec 2022. Is there anything pending for this bug report before closing as fixed?

Arun Giridhar <arungiridhar>
Group Member
Thu 08 Dec 2022 03:40:24 AM UTC, comment #6: 

ok. i trimmed out the note to just describe what's happening. 

pushing to octave 9 for now, as the consensus seems to be to keep 8.1 pretty locked down for now.

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

marking as ready for test.

Nicholas Jankowski <nrjank>
Group Member
Fri 02 Dec 2022 06:24:02 PM UTC, comment #5: 

Thanks, @nrjank. The FIXME note is just a copy of the corresponding FIXME note in octave_class::subsref (file ov-class.cc). I don't fully understand it, so I'm afraid I don't know if this is a partial fix of not.

Fernando <tutissanalio>
Mon 28 Nov 2022 10:11:04 PM UTC, comment #4: 

Also, Fernando, can you comment on the FIXME note left in the main function? Not quite clear if you consider this a partial fix, or if the fixme is specific to the MATLAB compatibility?

Nicholas Jankowski <nrjank>
Group Member
Mon 28 Nov 2022 09:17:43 PM UTC, comment #3: 

The attached patch still applies cleanly to either stable and default branches and passes a make check on default.

with the patch applied, the following test class:

classdef foo
  methods (Access = public)
    function varargout = subsref (x, idx)
      varargout = num2cell(zeros(size(idx(1).subs{1})));
    endfunction
  endmethods
endclassdef


now results in the following output:



>> x = foo
x =

  foo object with properties:


>> x{1:10}
ans = 0
ans = 0
ans = 0
ans = 0
ans = 0
ans = 0
ans = 0
ans = 0
ans = 0
ans = 0

>> y = {x{1:10}}
y =
{
  [1,1] = 0
  [1,2] = 0
  [1,3] = 0
  [1,4] = 0
  [1,5] = 0
  [1,6] = 0
  [1,7] = 0
  [1,8] = 0
  [1,9] = 0
  [1,10] = 0
}

>> clear y

>> [y{1:10}] = x{1:10}
y =
{
  [1,1] = 0
  [1,2] = 0
  [1,3] = 0
  [1,4] = 0
  [1,5] = 0
  [1,6] = 0
  [1,7] = 0
  [1,8] = 0
  [1,9] = 0
  [1,10] = 0
}


I've refreshed the patch and cleaned up the commit message. Unless someone sees any issue with the approach taken, i think this is ready to push to Octave 9 unless someone thinks the bugfix is worth pushing to 8.1 before release.

(file #54033)

Nicholas Jankowski <nrjank>
Group Member
Sat 05 Jun 2021 08:50:20 PM UTC, comment #2: 

Oops, I forgot the file! bug48693.patch

(file #51519)

Fernando <tutissanalio>
Sat 05 Jun 2021 08:45:34 PM UTC, comment #1: 

Here is a candidate patch that solves the issue. Basically, I just copied a few lines of code from the method octave_class:subsref to the method octave_classdef::subsref.

Fernando <tutissanalio>
Wed 03 Aug 2016 10:02:56 PM UTC, original submission:  

In a classdef handle class, the subsref method is supposed to be called with `nargout` set to the number of values that the indexing argument addresses.

This works for old-style struct-based classes, but not for classdef classes.

Given the following constructor


classdef foo < handle
endclassdef


and the following simple definition of subsref


function varargout = subsref (x, idx)
  varargout = num2cell (zeros (size (idx(1).subs{1})));
endfunction


Then the following only returns a single value, even though the indexing operation is over a range of 10 values, and nargout should be set accordingly to capture 10 values:


>> x = foo;
>> x{1:10}
ans = 0
>> y = {x{1:10}}
y =
{
  [1,1] = 0
}


but, assigning to the right number of output values gets the correct output


[y{1:10}] = x{1:10}


If foo is changed to be a struct-based class with the following constructor, but the subsref method is left exactly the same,


function y = foo ()
  y = class (struct (), "foo");
endfunction


then nargout is set correctly and the indexing examples work as expected.

This is a difference in how subsref is implemented in the octave_class vs octave_classdef classes.

Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #54033:  classdef_subsref_nargout_bug48693_v2.patch added by nrjank (3KiB - application/octet-stream - updated patch)
file #51519:  bug48693.patch added by tutissanalio (3KiB - text/x-patch)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by arungiridhar (Posted a comment)
  • -email is unavailable- added by nrjank (Updated the item)
  • -email is unavailable- added by tutissanalio (Posted a comment)
  •  

    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 group members can vote.

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-10-14 nrjank StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 9.1.0
    2023-02-27 rik5 Dependencies- bugs #63841 is dependent
    2022-12-08 nrjank StatusPatch Submitted Ready For Test
        Planned ReleaseNone 9.1.0
    2022-11-28 nrjank Attached File- Added classdef_subsref_nargout_bug48693_v2.patch, #54033
        CategoryInterpreter Classdef
        Item GroupMatlab Compatibility Incorrect Result
    2021-09-08 nrjank StatusNone Patch Submitted
    2021-06-05 tutissanalio Attached File- Added bug48693.patch, #51519
    2019-02-26 mtmiller Carbon-CopyRemoved 80942 -

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code