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

Submitted by:  Rik <rik5>
Submitted on:  Thu 10 Aug 2017 03:17:30 AM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Matlab Compatibility
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating 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 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>
Project 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>
Project Administrator
Thu 10 Aug 2017 08:21:50 PM UTC, comment #11:

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

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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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.

Rik <rik5>
Project 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:

and reintroduced here:

Adding Piotr to the notification list.

John W. Eaton <jwe>
Project 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:

But adding '()' makes the code work correctly

Rik <rik5>
Project Administrator

 

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

Attach File(s):
   
   
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
  • -unavailable- added by jsoh425 (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by jwe
  • -unavailable- added by rik5 (Submitted the item)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 11 Aug 2017 04:29:06 AM UTCrik5StatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Thu 10 Aug 2017 09:39:43 PM UTCjweStatusConfirmed=>Ready For Test
    Thu 10 Aug 2017 08:21:50 PM UTCjsoh425Attached File-=>Added patch_bug51715.diff, #41475
    Thu 10 Aug 2017 03:26:54 PM UTCrik5StatusNone=>Confirmed
    Thu 10 Aug 2017 02:56:50 PM UTCjweCarbon-Copy-=>Added jsoh425

    Back to the top


    Powered by Savane 3.1-cleanup1