bugGNU Octave - Bugs: bug #51532, Function in '+' package argument...

 
 

bug #51532: Function in '+' package argument lookup fails

Submitter:  Piotr Held <jsoh425>
Submitted:  Fri 21 Jul 2017 11:26:23 PM UTC
   
 
Category:  Interpreter Severity:  4 - Important
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Piotr Held 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

Mon 07 Aug 2017 11:58:52 PM UTC, comment #8: 

I made some tweaks and checked the change in here (http://hg.savannah.gnu.org/hgweb/octave/rev/6e0fd7e3c262).  Thanks for finding and fixing this issue.

Rik <rik5>
Group administrator
Mon 07 Aug 2017 10:08:05 PM UTC, comment #7: 

Sorry about that. Wasn't working on a clean copy when I made the changes. I applied the changes to a clean copy and tested it. Should be good now.

(file #41450)

Piotr Held <jsoh425>
Mon 07 Aug 2017 09:30:50 PM UTC, comment #6: 

@Piotr: Could you refresh your patch against a current development tip?  I tried to apply the patch and the code in pt-eval.cc was rejected by diff.

Rik <rik5>
Group administrator
Sat 05 Aug 2017 01:28:54 AM UTC, comment #5: 

Similarly to the recently added 'is_classdef_meta()' method I added a method 'is_package()' to octave_value, octave_base_value and octave_classdef_meta. It returns true if the cdef_classdef_meta object underlying octave_classdef_meta is a package.

I added some more commentary in pt-eval.cc:1347 and changed that area of code so that the the '+' package is not indexed.

I added a test for this problem in /test.

(file #41419)

Piotr Held <jsoh425>
Fri 04 Aug 2017 08:44:15 PM UTC, comment #4: 

I'm sorry about the confusion, it doesn't work. I'll try to work on a proper (nonhack) patch out quickly.

Piotr Held <jsoh425>
Fri 04 Aug 2017 08:39:32 PM UTC, comment #3: 

Currently (cset: 01899bdd2a3a) this functionality works.

Piotr Held <jsoh425>
Mon 24 Jul 2017 06:51:19 PM UTC, comment #2: 

Just for clarity this bug is a direct result of the solution to Bug #51192.

I am attaching a hack to make my tests pass and a proof-of-concept. However, I do not believe this is the proper implementation.

The way the hack works is it asks the interpreter (the one defined 'globally') what the load_path is and the asks the load_path whether a package that has the name: 'expr-&amp;gt;name()' exists. If yes then 'indexing_object = false'.

I do not exactly know how this should be implemented actually.
Should there be some code like meta.package in Matlab?

(file #41319)

Piotr Held <jsoh425>
Fri 21 Jul 2017 11:34:16 PM UTC, comment #1: 

Sorry about that.

When calling a function from a +package with arguments


&amp;gt;My_Package.foo('asd')


you get:


error: can't perform indexing operations for user-defined
function type


The problem seems to be linked to libinterp/parse-tree/pt-eval.cc:1346 in changeset 23781:d891b6a16a4d. When indexing_object is set to false during runtime the error does not occur.

It seems that the is_classdef_meta() is too broad of a check.

I attached in previous post a test for the problem.

Piotr Held <jsoh425>
Fri 21 Jul 2017 11:26:23 PM UTC, original submission:  

When calling

>My_Package.foo('arg')
+verbatim
the interpreter returns:
+verbatim+
error: can't perform indexing operations for user-defined function type
+verbatim+

Error seems to be in libinterp/parse-tree/pt-eval.cc:1346. If during runtime _indexing_object_ is changed from _true_ to _false_ then the code executes properly.

The

+verbatim+
is_classdef_meta()
+verbatim

check is too broad

Piotr Held <jsoh425>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #41450:  patch_bug51532.diff added by jsoh425 (5KiB - text/x-patch)
file #41419:  proper_patch.diff added by jsoh425 (5KiB - text/x-patch)
file #41319:  hack.diff added by jsoh425 (981B - text/x-patch - Hack of the bug, not an actual solution)
file #41284:  Package_Test.tar.gz added by jsoh425 (224B - application/gzip - The test that fails with this issue.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mtmiller (Updated the item)
  • -email is unavailable- added by jsoh425 (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-08-07 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2017-08-07 jsoh425 Attached File- Added patch_bug51532.diff, #41450
    2017-08-05 jsoh425 Attached File- Added proper_patch.diff, #41419
    2017-07-24 jsoh425 Attached File- Added hack.diff, #41319
    2017-07-24 mtmiller Severity3 - Normal 4 - Important
        StatusNone Confirmed
    2017-07-21 jsoh425 Attached File- Added Package_Test.tar.gz, #41284

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code