bugGNU Octave - Bugs: bug #47680, classdef @ operator to overrided...

 
 

bug #47680: classdef @ operator to overrided superclass methods

Submitted by:  Ivo <icouckuy>
Submitted on:  Tue 12 Apr 2016 05:57:49 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  Closed
Release:  dev Operating System:  GNU/Linux

Add a New Comment (Rich Markup)
   

You are not logged in

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

 

( Jump to the original submission)

Fri 22 Feb 2019 10:27:13 PM UTC, comment #26: 

The issue about the missed error when more values are passed to a function that it is declared to accept is a separate issue, so I'm closing this report as fixed.

John W. Eaton <jwe>
Project Administrator
Wed 20 Feb 2019 09:45:15 PM UTC, comment #25: 

You are right, I should have better said "must error to be compatible with Matlab" (what this bug report is about).

Personally, I don't need this error.  As you say, it is not used, thus cannot do any harm.  Maybe Matlab only errors because of the internal ways to handle such calls.

If one day Octave has a Linter, it could issue a warning about this circumstance...

Kai Torben Ohlhus <siko1056>
Project Member
Wed 20 Feb 2019 03:22:01 PM UTC, comment #24: 

No, it wasn't intentional to omit the argument from the superclass method.  But does it have to error if the argument isn't used in the function?  I believe that in Matlab, calling any function with more arguments than appear in the function definition is an error, but we don't do that automatically in Octave.  If you think we should, then I think that deserves a separate discussion.

John W. Eaton <jwe>
Project Administrator
Wed 20 Feb 2019 10:13:54 AM UTC, comment #23: 

Coming back to comment #19: Is the argument ('obj') omitted by intention in the definition of superclass.meth()?

When changing the code to

% superclass.m:
classdef superclass
  methods
    function meth (obj) % or alike
      'foo-i-hithere'
    end
  end
end

It runs fine in both Octave 6.0 and Matlab R2018b.

If the argument in superclass.meth() was omitted by intention, Octave should definitely raise an error because of the equivalence of

other_obj.meth@superclass ();
meth@superclass (other_obj);

https://www.mathworks.com/help/matlab/matlab_oop/method-invocation.html

Thus superclass.meth() must be able to handle a single input or error otherwise.

Kai Torben Ohlhus <siko1056>
Project Member
Thu 14 Feb 2019 01:54:43 PM UTC, comment #22: 

The w64-64 buildbot is happy with the change (and is failing later-on due to a known bug).
@jwe: Unrelated to the issue here, the w32 and w64-32 buildbots are failing because the gnulib sub-repos aren't clean since a few weeks. Could you please revert these sub-repos locally on the workers?

Markus Mützel <mmuetzel>
Project Member
Wed 13 Feb 2019 03:13:41 PM UTC, comment #21: 

I think the build problem should be fixed with this changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/456726d276a6

John W. Eaton <jwe>
Project Administrator
Wed 13 Feb 2019 08:12:45 AM UTC, comment #20: 

The buildbots for the Windows targets of the default branch fail with the following tail in the compile logs:

make  install-data-hook
make[5]: Entering directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
cat libinterp/dldfcn/PKG_ADD libgui/graphics/PKG_ADD > oct-file-pkg-add-t \
  && mv oct-file-pkg-add-t oct-file-pkg-add
/bin/mkdir -p /scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/usr/share/octave/6.0.0/etc
/bin/mkdir -p /scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/usr/lib/octave/6.0.0/oct/x86_64-pc-linux-gnu
/usr/bin/install -c -m 644 libinterp/DOCSTRINGS /scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/usr/share/octave/6.0.0/etc/built-in-docstrings
if [ -n "`cat libinterp/dldfcn/PKG_ADD libgui/graphics/PKG_ADD`" ]; then \
  /usr/bin/install -c -m 644 oct-file-pkg-add /scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/usr/lib/octave/6.0.0/oct/x86_64-pc-linux-gnu/PKG_ADD; \
fi
cd /scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/usr/lib/octave/6.0.0 && \
for ltlib in libinterp/dldfcn/__delaunayn__.la libinterp/dldfcn/__eigs__.la libinterp/dldfcn/__fltk_uigetfile__.la libinterp/dldfcn/__glpk__.la libinterp/dldfcn/__init_fltk__.la libinterp/dldfcn/__init_gnuplot__.la libinterp/dldfcn/__ode15__.la libinterp/dldfcn/__voronoi__.la libinterp/dldfcn/amd.la libinterp/dldfcn/audiodevinfo.la libinterp/dldfcn/audioread.la libinterp/dldfcn/ccolamd.la libinterp/dldfcn/chol.la libinterp/dldfcn/colamd.la libinterp/dldfcn/convhulln.la libinterp/dldfcn/dmperm.la libinterp/dldfcn/fftw.la libinterp/dldfcn/gzip.la libinterp/dldfcn/qr.la libinterp/dldfcn/symbfact.la libinterp/dldfcn/symrcm.la libgui/graphics/__init_qt__.la; do \
  f=`echo $ltlib | /bin/sed 's,.*/,,'`; \
  dl=`/bin/sed -n -e "s/dlname='\([^']*\)'/\1/p" < $f`; \
  if [ -n "$dl" ]; then \
    /usr/bin/install -c $dl /scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/usr/lib/octave/6.0.0/oct/x86_64-pc-linux-gnu/`echo $f | /bin/sed 's,^lib,,; s,\.la$,.oct,'`; \
  else \
    echo "error: dlname is empty in $ltlib!"; \
    exit 1; \
  fi; \
  lnames=`/bin/sed -n -e "s/library_names='\([^']*\)'/\1/p" < $f`; \
  if [ -n "$lnames" ]; then \
    rm -f $f $lnames $dl; \
  fi \
done
make[5]: Leaving directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make[4]: Leaving directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make[3]: Leaving directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make[2]: Leaving directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make[1]: Leaving directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make[1]: Entering directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make  dist-lzip dist-xz dist-gzip am__post_remove_distdir='@:'
make[2]: Entering directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make  distdir-am
make[3]: Entering directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
  MAKEINFO AUTHORS
  GEN      ChangeLog
  MAKEINFO INSTALL.OCTAVE
  MAKEINFO BUGS
/bin/mkdir -p doc/doxyhtml/pages
make[3]: *** No rule to make target 'test/bug-47680/sub_class47680.m', needed by 'distdir-am'.  Stop.
make[3]: *** Waiting for unfinished jobs....
cat ../README | /bin/sed '1s/.*/notitle {#mainpage}/; 2s/.*/=======/' > doc/doxyhtml/pages/README.md
make[3]: Leaving directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make[2]: *** [Makefile:27146: distdir] Error 2
make[2]: Leaving directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make[1]: *** [Makefile:27249: dist] Error 2
make[1]: Leaving directory '/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/octave-hg-repo/.build'
make: *** [/scratch/buildbot/workers/jwe-debian-x86_64-2/w64-64-on-debian/src/hg-octave-dist.mk:34: hg-octave-dist] Error 2
program finished with exit code 2
elapsedTime=1515.867216

Markus Mützel <mmuetzel>
Project Member
Mon 11 Feb 2019 09:35:39 PM UTC, comment #19: 

I pushed the following changesets related to this bug report to the default branch.  These changes will appear in Octave 6.

http://hg.savannah.gnu.org/hgweb/octave/rev/606605d0cd31
http://hg.savannah.gnu.org/hgweb/octave/rev/17e7d310def8
http://hg.savannah.gnu.org/hgweb/octave/rev/9b0335f4bc74
http://hg.savannah.gnu.org/hgweb/octave/rev/91b152968ec1

With the last changeset above, Octave should handle both of the following calls in the same way:

meth@superclass (obj, ...)
obj.meth@superclass (...)

As far as I can tell, the superclass name can include package qualifiers but I didn't add tests for that.

In comment #18, I showed the possibility of

pkg1.pkg2.cls.meth@pkg3.pkg4.supcls (...)

as possible.  But is it?  What is actually valid on the left of the '@' symbol in a call to a superclass method?

Also, the Matlab docs say that the method name in the superclass reference must match the name of the current function.  Does the object also have to be the same object for which the current method is called?  For example:

% superclass.m:
classdef superclass
  methods
    function meth ()
      'foo-i-hithere'
    end
  end
end

% myclass.m:
classdef myclass < superclass
  methods
    function meth (obj)
      other_obj = myclass ();
      other_obj.meth@superclass ();
    end
  end
end

% code
x = myclass ();
x.meth ();

This code currently works in Octave, but maybe it should be an error?

John W. Eaton <jwe>
Project Administrator
Mon 11 Feb 2019 04:32:42 PM UTC, comment #18: 

OK, I can see how to parse expressions like

obj.meth@supcls (...)
pkg1.pkg2.cls.meth@pkg3.pkg4.supcls (...);
...

with horizontal whitespace (but no newlines, so not arbitrary whitespace) surrounding the '@' or '.' characters.  This does break the handling of commands like

dbstop @ftp/ftp

but if I understand the comments to bug #46451, that kind of command doesn't work in Matlab either, so I would say we shouldn't worry about it, and instead fix our way of setting breakpoints in class methods to be complete and compatible with Matlab.

John W. Eaton <jwe>
Project Administrator
Fri 08 Feb 2019 07:16:21 AM UTC, comment #17: 

Yes, I mean that

this . addRequired @ inputParser( name, validator );

is interpreted as

this ('.', 'addRequired', '@', ... );

On the other, you are right, in the context of an assignment operator things are different.  More precisely,

x = this . addRequired @ inputParser( name, validator );

works exactly as if there were no spaces around the dot and the @-sign.

Julien Bect <jbect>
Thu 07 Feb 2019 09:22:17 PM UTC, comment #16: 

Why would "this" be interpreted as a function name in this context?  Do you mean as a "command-style" function?

What if you write

x = this . addRequired @ inputParser (name, validator);

?  It shouldn't be possible for it to be recognized as command syntax on the RHS of an assignment operator.

John W. Eaton <jwe>
Project Administrator
Thu 07 Feb 2019 09:09:12 PM UTC, comment #15: 

John,

Concerning the questions in Comment #14, I can answer for the first one.

Inserting spaces around the @ sign is ok:

this.addRequired @ inputParser( name, validator );

Inserting spaces around the dot is not ok:

this . addRequired @ inputParser( name, validator );

because then "this" is interpreted as a function name.

Julien Bect <jbect>
Thu 07 Feb 2019 08:23:24 PM UTC, comment #14: 

Admittedly, Matlab's OO was not very consistent (and not everything is documented as you found out). Especially when classdef was just introduced. E.g., static methods stopped working over versions, the rules on overloading private/protected methods changed all the time, etc.

Glad to see this being worked on.

-
Ivo

Ivo <icouckuy>
Thu 07 Feb 2019 08:11:00 PM UTC, comment #13: 

OK, maybe there is a better way to handle the parsing than what we are doing now that would simplify it a bit.

In superclass references and metaclass query expressions, are spaces allowed between the elements?  Can package names be used ot qualify the class names?  For example, are any of the following allowed (note spaces)?

obj . method @ superclass
pkg1 . pkg2 . class_name . method @ superclass
? class_name
? pkg1 . pkg2 . class_name

If spaces are allowed, then I think we already get some of these wrong.  And we try to recognize hings like obj@superclass as a single identifier in the lexer instead of just returning a series of tokends (ID, '@', ID) and recognizing the syntax in the parser.

John W. Eaton <jwe>
Project Administrator
Thu 07 Feb 2019 07:48:59 PM UTC, comment #12: 

Yes, the example provided by Ivo works fine R2018b.

Julien Bect <jbect>
Thu 07 Feb 2019 02:08:51 PM UTC, comment #11: 

Julien:  Yes, but in the pages that you linked, I don't see any example or statement showing that

obj.method@superclass (args, ...)

will work.  Since these are always invoked from inside subclass methods or constructors, I don't think the limitation in syntax would be much of a problem in practice.  I'm simply asking for someone to verify whether the syntax above actually does work in Matlab.

I need to know whether it is actually valid Matlab syntax.  If it is not, then then I won't waste time trying to figure out how to properly parse it in Octave.

John W. Eaton <jwe>
Project Administrator
Thu 07 Feb 2019 07:46:17 AM UTC, comment #10: 

In my opinion this is a general feature of Matlab OOP, not a special syntax from this particular syntax.

What I mean is that

obj.XXXX (args, ...)

is supposed to be equivalent to

XXXX (obj, args ...)

for any method XXXX.  Taking XXXX to method@superclass yields (A) and (B) in Comment #9.

https://fr.mathworks.com/help/matlab/matlab_oop/specifying-methods-and-functions.html

The only special thing here is that method@superclass is only visible from method in the subclass.

https://fr.mathworks.com/help/matlab/matlab_oop/calling-superclass-methods-on-subclass-objects.html

Julien Bect <jbect>
Thu 07 Feb 2019 04:16:39 AM UTC, comment #9: 

Reposing to fix botched verbatim tag.

I started looking at this bug report and I don't see where the syntax

obj.method@superclass (args, ...);  %% (A)

is documented as a way to call superclass methods in Matlab. All examples that I've found use the syntax

method@superclass (obj, args, ...) %% (B)

Is the syntax (A) really allowed by Matlab? Could someone please verify?

John W. Eaton <jwe>
Project Administrator
Thu 07 Feb 2019 04:14:21 AM UTC, comment #8: 

I started looking at this bug report and I don't see where the syntax

obj.method@superclass (args, ...);  %% (A)

is documented as a way to call superclass methods in Matlab.  All examples that I've found use the syntax

+vertabim+
method@superclass (obj, args, ...)  %% (B)
-verbatim-

Is the syntax (A) really allowed by Matlab?  Could someone please verify?

John W. Eaton <jwe>
Project Administrator
Wed 29 Jun 2016 10:27:14 AM UTC, comment #7: 

Marking as "confirmed" and retagging "Matlab compatibility" instead of "Regression", since there is no evidence it worked previously.

Lachlan Andrew <lachlan>
Tue 17 May 2016 12:23:42 PM UTC, comment #6: 

The status can be set to "Confirmed".

I don't know if "Regression" is the appropriate item group.

Julien Bect <jbect>
Tue 17 May 2016 12:18:56 PM UTC, comment #5: 

I can confirm that the problem also exists on the tip of branch 'default'.

Julien Bect <jbect>
Mon 16 May 2016 04:19:21 PM UTC, comment #4: 

Hi Ivo,

I can confirm the problem in Octave 4.0.2 (actually, even on the tip of branch 'stable').

I will compile the latest development version (tip of branch 'default') and see if the problem is there too.

@++
Julien

Julien Bect <jbect>
Wed 13 Apr 2016 07:59:48 AM UTC, comment #3: 

Not sure, maybe "syntax error calling overrided method using this.method@super(x)"

Ivo <icouckuy>
Tue 12 Apr 2016 07:14:05 PM UTC, comment #2: 

What would you want the title to read? I can change it for you

Philip Nienhuis <philipnienhuis>
Project Member
Tue 12 Apr 2016 05:59:46 PM UTC, comment #1: 

Sorry, title isn't really that informative. But I don't seem to be able to change it anymore

Ivo <icouckuy>
Tue 12 Apr 2016 05:57:49 PM UTC, original submission:  

The attached class fails to construct due to syntax error on octave 4.0.1.

>> myInputParser

parse error near line 6 of file /home/ilm/projecten/myInputParser.m

  syntax error

>>>             this.addRequired@inputParser( this, name, validator );


Replacing that line by
addRequired@inputParser(this, name, validator)
fixes it.

Ivo <icouckuy>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36911:  myInputParser.m added by icouckuy (392B - text/x-matlab)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by jbect (Posted a comment)
  • -email is unavailable- added by icouckuy (Submitted the item)
  •  

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

    Only project members can vote.

     

     

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-02-22 jwe StatusReady For Test => Fixed
        Open/ClosedOpen => Closed
    2019-02-11 jwe StatusConfirmed => Ready For Test
    2016-06-29 lachlan Item GroupRegression => Matlab Compatibility
        StatusNone => Confirmed
        Release4.0.1 => dev
    2016-04-12 icouckuy Attached File- => Added myInputParser.m, #36911

    Back to the top


    Powered by Savane 3.5