bugGNU Octave - Bugs: bug #49169, overriding @super/class method,...

 
 

bug #49169: overriding @super/class method, builtin class can't be called from @sub/sub constructor

Submitted by:  Ll <pipeng>
Submitted on:  Fri 23 Sep 2016 08:24:11 PM UTC  
 
Category:  Octave Function Severity:  3 - Normal
Priority:  3 - Low Item Group:  Matlab Compatibility
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  dev Operating System:  Any

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)

Tue 27 Sep 2016 10:48:12 PM UTC, comment #14: 

My opinion is that having `builtin("class"...)` working is more important than preventing people shooting themselves in the foot.  That is, just doing the `called_from_builtin()` thing you mention sounds sufficient to me.  (Just my $0.02.)

Colin Macdonald <cbm>
Tue 27 Sep 2016 09:53:51 PM UTC, comment #13: 

Ok, so it's clear that we should allow class() to be called via builtin() from a class ctor function.

I don't know enough about how to traverse the Octave call stack to implement this. It is easy enough to call called_from_builtin() to detect that class() was called by builtin, but I don't easily see how to get a reference to one higher in the call stack to make sure we are in a class ctor of the same name as the 2nd argument.

If someone can supply a patch to do this, I'll be happy to test and apply.

Mike Miller <mtmiller>
Project Administrator
Tue 27 Sep 2016 09:24:26 PM UTC, comment #12: 

only remeber you only can use class to set values inside the same type or you will get the 'is not a valid class name', @foo/foo only can use foo to set the class.

Ll <pipeng>
Tue 27 Sep 2016 08:48:55 PM UTC, comment #11: 

I also tried making a `foo` from within some unrelated class (`baz`).  This also doesn't work:

>> y = baz()
Error using class
The name 'foo' is not a valid class name.

Error in baz (line 4)
  x = builtin ('class', x, 'foo', bar);

This language is restricting my freedom to make dumb choices ;-) j/k!

Maybe it would work if the @foo class were nested inside the `@baz` directory: I didn't try that!

Colin Macdonald <cbm>
Tue 27 Sep 2016 08:45:29 PM UTC, comment #10: 

Surprisingly (to me), that is indeed an error:

>> builtin('class', struct(), 'foo', bar)
Error using class
The CLASS function must be called from a class constructor.

I can see why its a bad idea, but I don't see why its an error: I must be programming too much Python

Colin Macdonald <cbm>
Tue 27 Sep 2016 07:34:14 PM UTC, comment #9: 

And if builtin('class', struct(), classname, ...) is called from a non-class method or from the top-level does Matlab detect and report an error?

Mike Miller <mtmiller>
Project Administrator
Tue 27 Sep 2016 06:46:20 PM UTC, comment #8: 

s/maybe that should work/that should work/

;-)

Colin Macdonald <cbm>
Tue 27 Sep 2016 06:44:34 PM UTC, comment #7: 

The end of your comment was cut off by markup processor:

  function x = foo()
   x.value = bar;
-  x = class (x, "foo", bar);
+  x = builtin ("class", x, "foo", bar);
endfunction

Yes, maybe that should work. The simple check in place now only makes sure that the direct caller in the call stack is a class constructor.

Mike Miller <mtmiller>
Project Administrator
Tue 27 Sep 2016 06:43:33 PM UTC, comment #6: 

Looks like something ate my verbatim in a previous comment.

What should work (and does in Matlab) is:

x = builtin ('class', x, 'foo', bar);

(also it would appear that verbatim's cannot contain lines that start with +/- like diff output).

Colin Macdonald <cbm>
Tue 27 Sep 2016 06:42:24 PM UTC, comment #5: 

Matlab:

>> type @bar/class

function s = class (x, varargin)
  error ('bar.class: this should not be called');
end
>> f = foo()
Error using bar/class (line 2)
bar.class: this should not be called

Error in foo (line 3)
  x = class (x, 'foo', bar);

And it works using `builtin`...

Colin Macdonald <cbm>
Tue 27 Sep 2016 06:31:57 PM UTC, comment #4: 

I agree that should be expected behaviour (I have also not tested on Matlab).

But what might be expected to work is the following change to your MWE:

Colin Macdonald <cbm>
Tue 27 Sep 2016 06:16:19 PM UTC, comment #3: 

Ok, so that error is specifically about a constructor using inheritance from another object type that overrides the class method. I can confirm with a minimum working example:

>> type @bar/bar

function b = bar ()
  b = class (struct (), "bar");
endfunction

>> type @bar/class

function class (x)
  error ("bar.class: this should not be called");
endfunction

>> type @foo/foo

function x = foo ()
  x.value = bar;
  x = class (x, "foo", bar);
endfunction

>> x = foo
error: bar.class: this should not be called
error: called from
    class at line 2 column 3
    foo at line 3 column 5

The override behavior seems to be fairly consistent with other methods in Octave. It would be good to find out what Matlab does with an example like this before we try to guess what the right behavior should be.

Mike Miller <mtmiller>
Project Administrator
Tue 27 Sep 2016 01:12:18 AM UTC, comment #2: 

Hi Mike Miller, nice to see you again, this is where this come:

https://github.com/cbm755/octsympy/pull/561

There we need two things, first set class, and get class, there you can see what happen in all this cases.

Thx.

Ll <pipeng>
Mon 26 Sep 2016 09:24:01 PM UTC, comment #1: 

This error is probably because the class function knows whether it is being called from a class constructor function or not. If you call it using builtin, that adds an indirection to the stack and it no longer knows that it is in a constructor context.

However, you don't need to use builtin to do what you want. I made a simple example to show the class I think you are describing:

>> type @foo/foo

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


>> type @foo/class

function s = class (x)
  s = "foo-type";
endfunction

>> x = foo;
>> class (x)
ans = foo-type

It seems to work for me without needing to use builtin, probably because Octave defers calling overridden methods in the constructor.

Is there something missing in your example that shows why you need to use builtin and why you think that should work as you describe?

Do you have access to Matlab to compare and show whether they work one way or the other?

Mike Miller <mtmiller>
Project Administrator
Fri 23 Sep 2016 08:24:11 PM UTC, original submission:  

As the title says, when we use the class function to set the class works fine in @foo/foo.m, but if we instead call it directly we call it with the builtin function send the error:

error: class: 'foo' is invalid as a class name in this context
error: called from
    foo at line 184 column 5

only to be clear, this works:

class(f, 'foo', expr)

this don't works:

builtin('class', f, 'foo', expr)

Why we can need call class with builtin?, when we overload the class function of @foo only to get data instead of set.
To others examples imagine!

Thx. Cya.

Ll <pipeng>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by cbm (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by pipeng (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
    2016-09-27 mtmiller StatusNeed Info => Confirmed
    2016-09-27 mtmiller Operating SystemGNU/Linux => Any
        SummaryClass can't be called from builtin function. => overriding @super/class method, builtin class can't be called from @sub/sub constructor
    2016-09-27 mtmiller Priority5 - Normal => 3 - Low
        Item GroupNone => Matlab Compatibility
        Release4.0.3 => dev
    2016-09-26 mtmiller StatusNone => Need Info

    Back to the top


    Powered by Savane 3.5