Skip to content

Commit 8d6672c

Browse files
author
Fernando Alvarruiz
committed
Don't always call numArgumentsFromSubscript in classdef susbsref (bug #67403).
* ov-classdef.cc (octave_classdef::subsref): Only call numArgumentsFromSubscript if nargout<=0 (not an assignment) and the last index type is not '()'. Change maybe_cs_list_query variable by the condition that last index type is not '()'. * test/bug-67403/class_bug67403.m: Add class for test. * test/bug-67403/bug-67403.tst: Add test. * test/bug-67403/module.mk: Add new files to build system.
1 parent c28ddac commit 8d6672c

File tree

4 files changed

+68
-38
lines changed

4 files changed

+68
-38
lines changed

libinterp/octave-value/ov-classdef.cc

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -489,46 +489,50 @@ octave_classdef::subsref (const std::string& type,
489489
m_count++;
490490
args(0) = octave_value (this);
491491

492-
octave::cdef_method meth_nargout
493-
= cls.find_method ("numArgumentsFromSubscript");
494-
if (meth_nargout.ok ())
492+
if (nargout <= 0)
495493
{
496-
octave_value_list args_nargout (3);
497-
498-
args_nargout(0) = args(0);
499-
args_nargout(1) = args(1);
500-
// FIXME: Third argument should be one of the possible values of
501-
// the matlab.mixin.util.IndexingContext enumeration class.
502-
args_nargout(2) = octave_value (Matrix ());
503-
retval = meth_nargout.execute (args_nargout, 1, true,
504-
"numArgumentsFromSubscript");
505-
506-
nargout = retval(0).strict_int_value
507-
("subsref: return value of 'numArgumentsFromSubscript' must be integer");
508-
}
509-
else if (nargout <= 0)
510-
{
511-
// If the number of output arguments is unknown, attempt to set up
512-
// a proper value for nargout at least in the simple case where the
513-
// cs-list-type expression - i.e., {} or ().x, is the leading one.
514-
bool maybe_cs_list_query = (type[0] == '.' || type[0] == '{'
515-
|| (type.length () > 1 && type[0] == '('
516-
&& type[1] == '.'));
517-
518-
if (maybe_cs_list_query)
494+
// If the last index type is not '()', the final value of nargout is
495+
// unknown. Try to get its value
496+
if (type.back () != '(')
519497
{
520-
// Set up a proper nargout for the subsref call by calling numel.
521-
octave_value_list tmp;
522-
int nout;
523-
if (type[0] != '.')
524-
tmp = idx.front ();
525-
526-
nout = xnumel (tmp);
527-
// Take nout as nargout for subsref, unless the index expression
528-
// is a whole sentence starting with the form id.member and id is
529-
// one element (in that case, nargout remains 0).
530-
if (type[0] != '.' || nout != 1 || nargout < 0)
531-
nargout = nout;
498+
// See if method numArgumentsFromSubscript is defined
499+
octave::cdef_method meth_nargout
500+
= cls.find_method ("numArgumentsFromSubscript");
501+
502+
if (meth_nargout.ok ())
503+
{
504+
octave_value_list args_nargout (3);
505+
506+
args_nargout(0) = args(0);
507+
args_nargout(1) = args(1);
508+
// FIXME: Third argument should be one of the possible values of
509+
// the matlab.mixin.util.IndexingContext enumeration class.
510+
args_nargout(2) = octave_value (Matrix ());
511+
retval = meth_nargout.execute (args_nargout, 1, true,
512+
"numArgumentsFromSubscript");
513+
514+
nargout = retval(0).strict_int_value
515+
("subsref: return value of 'numArgumentsFromSubscript' must be integer");
516+
}
517+
else
518+
{
519+
// Method numArgumentsFromSubscript undefined. Attempt to set up
520+
// a proper value for nargout at least in the simple case where the
521+
// cs-list-type expression - i.e., {} or ().x, is the leading one.
522+
523+
// Set up a proper nargout for the subsref call by calling numel.
524+
octave_value_list tmp;
525+
int nout;
526+
if (type[0] != '.')
527+
tmp = idx.front ();
528+
529+
nout = xnumel (tmp);
530+
// Take nout as nargout for subsref, unless the index expression
531+
// is a whole sentence starting with the form id.member and id is
532+
// one element (in that case, nargout remains 0).
533+
if (type[0] != '.' || nout != 1 || nargout < 0)
534+
nargout = nout;
535+
}
532536
}
533537
else if (nargout < 0)
534538
nargout = 1;

test/bug-67403/bug-67403.tst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
%!test <*67403>
2+
%! cb = class_bug67403 ();
3+
%! # Check numArgumentsFromSubscript is not called
4+
%! [a, b, c] = cb (1);
5+
%! assert ([a, b, c], [3, 3, 3]);
6+
%! assert (cb(1:3), 1);
7+
%! assert (cb{1}.a(1:3), 1);
8+
%! # Check numArgumentsFromSubscript is called
9+
%! assert ([cb{1:10}], [2, 2]);
10+
%! assert ([cb.a], [2, 2]);

test/bug-67403/class_bug67403.m

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
classdef class_bug67403
2+
methods
3+
function [varargout] = subsref (obj, s)
4+
varargout = cell(1, max (nargout, 1));
5+
varargout(:) = {nargout};
6+
endfunction
7+
function n = numArgumentsFromSubscript(obj, s, indexingContext)
8+
n = 2;
9+
endfunction
10+
endmethods
11+
endclassdef

test/bug-67403/module.mk

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
bug_67403_TEST_FILES = \
2+
%reldir%/bug-67403.tst \
3+
%reldir%/class_bug67403.m
4+
5+
TEST_FILES += $(bug_67403_TEST_FILES)

0 commit comments

Comments
 (0)