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

 
 

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

Submitted by:  Piotr Held <jsoh425>
Submitted on:  Fri 21 Jul 2017 11:26:23 PM UTC  
 
Category: InterpreterSeverity: 4 - Important
Priority: 5 - NormalItem Group: Regression
Status: FixedAssigned to: None
Originator Name: Piotr HeldOpen/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)

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

you get:

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

Piotr Held <jsoh425>

 

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

Attach File(s):
   
   
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
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by mtmiller (Updated the item)
  • -unavailable- added by jsoh425 (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 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 07 Aug 2017 11:58:52 PM UTCrik5StatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Mon 07 Aug 2017 10:08:05 PM UTCjsoh425Attached File-=>Added patch_bug51532.diff, #41450
    Sat 05 Aug 2017 01:28:54 AM UTCjsoh425Attached File-=>Added proper_patch.diff, #41419
    Mon 24 Jul 2017 06:51:19 PM UTCjsoh425Attached File-=>Added hack.diff, #41319
    Mon 24 Jul 2017 06:32:56 PM UTCmtmillerSeverity3 - Normal=>4 - Important
      StatusNone=>Confirmed
    Fri 21 Jul 2017 11:26:23 PM UTCjsoh425Attached File-=>Added Package_Test.tar.gz, #41284

    Back to the top


    Powered by Savane 3.1-cleanup1