bugGNU Octave - Bugs: bug #41723, Local functions in classdef files

 
 

bug #41723: Local functions in classdef files

Submitter:  Guillaume <gyom>
Submitted:  Thu 27 Feb 2014 03:52:06 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Guillaume Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 09 Apr 2017 12:03:33 AM UTC, comment #13: 

Thanks Guillaume, I merged your suggestions into my test case, pushed

https://hg.savannah.gnu.org/hgweb/octave/rev/67a638ac7f24

Mike Miller <mtmiller>
Group Member
Wed 05 Apr 2017 10:41:16 AM UTC, comment #12: 

I modified my classdef files to work around bug #50716 and I can confirm everything works fine so this report can be closed as fixed after pushing Mike's test case.

I just suggest a slightly modified test using this:


classdef class_bug41723 < handle
  properties
    value
  endproperties
  methods
    function obj = class_bug41723 ()
      obj.value = helper_function ();
    endfunction

    function val = getval (obj)
      val = helper_function_2 ();
    endfunction
  endmethods
endclassdef

function y = helper_function ()
  y = 42;
endfunction

function y = helper_function_2 (in)
  if (! nargin)
    y = helper_function_2 (true);
  else
    y = 2 * helper_function ();
  endif
endfunction


and add this case:


assert (obj.getval (), 2 * obj.value);


so that it also tests local functions calling themselves or another local function, as well as being called from a method and not just the constructor.

Many thanks for this!

Guillaume <gyom>
Tue 04 Apr 2017 02:47:59 PM UTC, comment #11: 

First things first, thank you very much for your changeset!

From my initial tests, it seems to work great. There isn't a parse error for the classdef files I use any more and local functions are usable.

I am limited with further testing of existing classdef files (that use local functions called recursively, or by other local functions, or by methods (public or private), etc) by a new bug that I unearthed in the process (bug #50716).

Guillaume <gyom>
Tue 04 Apr 2017 04:15:52 AM UTC, comment #10: 

I made a very basic test case in the attached. I wonder if Guillaume or someone might want to suggest additional things to test for before pushing this.

(file #40255)

Mike Miller <mtmiller>
Group Member
Mon 03 Apr 2017 10:12:12 PM UTC, comment #9: 

I checked in the following changeset.

  http://hg.savannah.gnu.org/hgweb/octave/rev/49f051ef6f2f

All existing tests still pass and my minimal tests show that it works, at least when calling a file-local function from a classdef constructor or method.

Could someone who needs this feature come up with some new test cases specifically for it?

John W. Eaton <jwe>
Group administrator
Wed 29 Mar 2017 03:57:46 PM UTC, comment #8: 

Last time I heard of something like this, it took 358 years of effort to solve. :-)

I'm looking forward to it, it will be an important step towards adoption of classdef in Octave. Thanks!

Guillaume <gyom>
Fri 24 Mar 2017 07:54:47 PM UTC, comment #7: 

Guillaume: after our discussion at OctConf, I took another look at this.  The parsing issue is not too difficult.  But while looking at where to store the functions in the symbol table handle the details of properly searching for them, I came to the conclusion that the symbol table code could really benefit from another round of pretty serious refactoring.  I have some good ideas on this, but they are too long to write in the margin of a comment box.  :-)

But seriously, I will try to do something about this soon as I think the symbol table code has become far too complicated and it seems there must be a better way.

John W. Eaton <jwe>
Group administrator
Wed 08 Feb 2017 06:53:47 AM UTC, comment #6: 

Chebfub package (http://www.chebfun.org) makes extensive use of classdef classes and local functions in classdef files.

I am asking again the question in comment #5:
"How difficult would it be to fix the parsing error first?"


Avinoam Kalma <avinoam>
Group Member
Mon 30 Jan 2017 10:31:30 AM UTC, comment #5: 

Being able to define local functions in classdef files is quite useful when classes become more complicated: how difficult would it be to fix the parsing error first, even if local functions are not dealt with properly to start with?

Guillaume <gyom>
Wed 05 Mar 2014 04:00:47 PM UTC, comment #4: 

John's right. This construct is not supported yet.

Michael Goffioul <goffioul>
Wed 05 Mar 2014 03:56:03 PM UTC, comment #3: 

I'm pretty sure the issue is that Octave is not expecting there to be anything but comments and whitespace in the file after the END that matches CLASSDEF.

I didn't know that classdef files could define any functions outside of the classdef block.

Fixing the parsing problem will be fairly simple.  Fixing the lookup rules so that these functions behave as private functions that are only visible to other functions defined in the classdef file will probably require more work.


John W. Eaton <jwe>
Group administrator
Wed 05 Mar 2014 03:46:27 PM UTC, comment #2: 

Hi Felipe,
I removed that line from the MathWorks example beforehand so I'm afraid that's not the explanation.
Thanks.

Guillaume <gyom>
Wed 05 Mar 2014 08:10:07 AM UTC, comment #1: 

could it be the ellipsis on that line?

Felipe G. Nievinski <fgnievinski>
Thu 27 Feb 2014 03:52:06 PM UTC, original submission:  

There is a parse error when using a classdef file that contains local functions, as described here:
http://www.mathworks.com/help/matlab/matlab_oop/specifying-methods-and-functions.html#br2la89


octave> t=MyClass
parse error near line 11 of file MyClass.m

  syntax error

>>> function myUtilityFcn
           ^


Guillaume <gyom>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #40255:  test-case-41723.diff added by mtmiller (2KiB - text/x-patch)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by avinoam (Posted a comment)
  • -email is unavailable- added by amro_octave
  • -email is unavailable- added by cbm
  • -email is unavailable- added by mtmiller (Updated the item)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by fgnievinski (Posted a comment)
  • -email is unavailable- added by gyom (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 group members can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-04-09 mtmiller StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2017-04-04 mtmiller Attached File- Added test-case-41723.diff, #40255
    2017-04-03 jwe StatusConfirmed Ready For Test
    2016-12-09 amro_octave Carbon-Copy- Added amro_octave
    2015-03-20 jordigh Dependencies- Depends on bugs #44590
    2014-08-22 cbm Carbon-Copy- Added cbm
    2014-03-27 mtmiller CategoryNone Interpreter
        StatusNone Confirmed
    2014-03-05 gyom Carbon-Copy- Added goffioul

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code