bugGNU Octave - Bugs: bug #51715, pkg.fcn should call function fcn...

 
 

bug #51715: pkg.fcn should call function fcn with no arguments for Matlab compatibility

Submitter:  Rik <rik5>
Submitted:  Thu 10 Aug 2017 03:17:30 AM 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:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 11 Aug 2017 04:29:06 AM UTC, comment #14: 

I checked, and the original code that motivated the report now runs correctly.  Marking as fixed and closing report.

Rik <rik5>
Group administrator
Thu 10 Aug 2017 09:45:47 PM UTC, comment #13: 

I'm sorry about that. I know that Octave is 2 whitespace and the reasoning behind it.

Piotr Held <jsoh425>
Thu 10 Aug 2017 09:39:43 PM UTC, comment #12: 

I pushed the changeset here:

http://hg.savannah.gnu.org/hgweb/octave/rev/5352f632859b

For the future, we try to avoid TAB characters in the Octave sources.  Can you configure your editor to use spaces instead of TABs when editing Octave sources?

Thanks.

Meanwhile, I also noticed that my changeset to add the predicates for classdef_superclass_ref objects included an unintended change.  I undid that in another changeset.

John W. Eaton <jwe>
Group administrator
Thu 10 Aug 2017 08:21:50 PM UTC, comment #11: 

So I screwed up when doing my cset. I deleted by mistake:


beg = n;
idx.clear ();


Sorry about that.
I am attaching a cset with the correction.

(file #41475)

Piotr Held <jsoh425>
Thu 10 Aug 2017 05:07:07 PM UTC, comment #10: 

I second that.  Virtual functions usually are implemented as a jump table so while there is one level of pointer indirection, its still very efficient.  On the other hand, dynamic_cast requires the runtime environment to do whatever is necessary to make sure that the object to be converted results in a valid object of the cast type.  This might mean initializing new member variables, replacing a virtual function with a new implementation, etc.  The performance penalty is not well-bounded.

Rik <rik5>
Group administrator
Thu 10 Aug 2017 04:33:43 PM UTC, comment #9: 

The predicates also involve virtual function calls so there are performance considerations there too.  But I'd just generally prefer to avoid casts.  Or to refactor so that directly inquiring about types isn't necessary.  But for now, let's stick with the current structure of the code if we can.  I'd of course be happy to make it all simpler, but that might not be possible.

John W. Eaton <jwe>
Group administrator
Thu 10 Aug 2017 04:31:50 PM UTC, comment #8: 

Maybe I was wrong. I'll look at it in a couple hours.

Piotr Held <jsoh425>
Thu 10 Aug 2017 04:30:17 PM UTC, comment #7: 

Piotr: That doesn't seem to fix the problem or I misunderstood the change you were suggesting.

I checked in the following changeset to add predicates for classdef_superclass_ref objects:

http://hg.savannah.gnu.org/hgweb/octave/rev/d56c18dc1373

John W. Eaton <jwe>
Group administrator
Thu 10 Aug 2017 04:16:08 PM UTC, comment #6: 

Roger that.

Piotr Held <jsoh425>
Thu 10 Aug 2017 04:15:13 PM UTC, comment #5: 

Given the frequency that statements are evaluated, I would also be concerned with any cast which might impair performance.

Rik <rik5>
Group administrator
Thu 10 Aug 2017 04:06:41 PM UTC, comment #4: 

Rik: I added a test:

http://hg.savannah.gnu.org/hgweb/octave/rev/e660b93d51dc

Piotr: I generally prefer to avoid casts if possible.

John W. Eaton <jwe>
Group administrator
Thu 10 Aug 2017 03:59:13 PM UTC, comment #3: 

About this bug: I figure (and haven't checked this) that all that is needed is a is_classdef_superclass_ref () check at here

Should that check be on octave_value level, or octave_function level?

The reason I'm asking is that octave_value has so many is_... checks that I was wondering if it would not be enough to put it on octave_function level and then dynamic cast to it to check. On the other hand, you get a slight performance hit when you do that. Any thoughts?

Piotr Held <jsoh425>
Thu 10 Aug 2017 03:26:54 PM UTC, comment #2: 

I also propose adding the following BIST test somewhere so this doesn't keep happening.


%!test
%! x = containers.Map;
%! assert (isobject (x));




Rik <rik5>
Group administrator
Thu 10 Aug 2017 02:56:50 PM UTC, comment #1: 

This problem was introduced when I refactored the evaluator.  Then it was fixed here:


changeset:   23690:ffd27f53fc79
user:        John W. Eaton <jwe@octave.org>
date:        Fri Jun 23 18:18:07 2017 -0400
summary:     make +package function calls work again (bug #51295, #51296)


and reintroduced here:


changeset:   23849:ff893e26aeeb
user:        Piotr Held <pjheld@gmail.com>
date:        Tue Aug 08 18:18:01 2017 -0400
summary:     make calling parent class constructors work again (bug #51534)


Adding Piotr to the notification list.

John W. Eaton <jwe>
Group administrator
Thu 10 Aug 2017 03:17:30 AM UTC, original submission:  

Without parentheses '()' to help the interpreter, Octave issues an invalid index error.

Example Code:


x = containers.Map
error: invalid index type '.' for function call


But adding '()' makes the code work correctly


x = containers.Map ()
x =

  containers.Map object with properties:

    Count     : 0
    KeyType   : char
    ValueType : any




Rik <rik5>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #41475:  patch_bug51715.diff added by jsoh425 (975B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jsoh425 (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by jwe
  • -email is unavailable- added by rik5 (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-08-11 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2017-08-10 jwe StatusConfirmed Ready For Test
    2017-08-10 jsoh425 Attached File- Added patch_bug51715.diff, #41475
    2017-08-10 rik5 StatusNone Confirmed
    2017-08-10 jwe Carbon-Copy- Added jsoh425

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code