bugGNU Octave - Bugs: bug #60845, Variables in nested functions...

 
 

bug #60845: Variables in nested functions exhibit multiple problems

Submitter:  José Matos <jamatos>
Submitted:  Mon 28 Jun 2021 07:54:30 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 05 Jul 2021 03:47:34 PM UTC, comment #17: 

I made jwe's diff from comment #15 into a changeset with a commit message and checked it in under his name here: http://hg.savannah.gnu.org/hgweb/octave/rev/2bb72743d3eb.

Marking bug as fixed and closing report.

Rik <rik5>
Group administrator
Sat 03 Jul 2021 04:39:49 PM UTC, comment #16: 

@jwe: tested the patch from comment #15 and it works in the cases I tried.  I think you can go ahead and commit it.

Rik <rik5>
Group administrator
Fri 02 Jul 2021 09:47:32 PM UTC, comment #15: 

Rik: I think the attached change fixes the problem.



(file #51641)

John W. Eaton <jwe>
Group administrator
Fri 02 Jul 2021 05:00:22 PM UTC, comment #14: 

@jwe: What was the result of compatibility tests with Matlab?  It appears that `who` does consolidate names and not list duplicates, at least as I read the examples on Discourse.

Rik <rik5>
Group administrator
Tue 29 Jun 2021 06:09:11 PM UTC, comment #13: 

@jwe: Makes sense.  Given that Discourse moves an old thread to the top of the display when there has been a new posting this should work well.

Rik <rik5>
Group administrator
Tue 29 Jun 2021 04:26:57 PM UTC, comment #12: 

Rik:  I posted a question here:

https://octave.discourse.group/t/need-help-understanding-matlab-behavior/1313

I propose that we just reuse this new topic for any future questions like this.  Otherwise we just end up with N discourse topics with nearly the same subject line.

John W. Eaton <jwe>
Group administrator
Tue 29 Jun 2021 03:47:27 PM UTC, comment #11: 

Now that Octave Maintainer's mailing list is not really used, where should we post requests for compatibility tests?  Over on Discourse?

I don't think the bug tracker gets enough eyes viewing individual bug reports to notice your request in comment #9.

Rik <rik5>
Group administrator
Tue 29 Jun 2021 10:57:19 AM UTC, comment #10: 
John W. Eaton <jwe>
Group administrator
Tue 29 Jun 2021 09:15:46 AM UTC, comment #9: 

I think the reason the behavior seemed so strange before the patch is that variable values in a stack frame are accessed by an offset into an array.  If the offsets are wrong the results appear random.  In this case, it seems that the offsets were computed correctly (looking at the correct parent function for non-local variables) but then applied to the wrong stack frame (the sibling nested function instead of the parent).

Regarding the duplicate names in the list produced by "who", can someone check what a recent version of Matlab does?  I remember asking about this behavior when I was working on nested functions in the last couple of years and finding that it also displays duplicates but I can't find the test cases and output now.

As you noticed, the output of "whos" makes a little more sense.  As far as I know, "who" just goes through the scopes/stack frames in the same way and gathers all the names.  I agree that having duplicate names in the output of "who" is strange, but if that is what Matlab does, I'm somewhat inclined to leave it alone.

In any case, I will make the change on stable and merge with default.

John W. Eaton <jwe>
Group administrator
Mon 28 Jun 2021 10:35:20 PM UTC, comment #8: 

Looks like the fix for duplicate variables is going to have to take place in libinterp/corefcn/stack-frame.cc.

Rik <rik5>
Group administrator
Mon 28 Jun 2021 10:16:47 PM UTC, comment #7: 

Using jwe's patch posted over on Discourse (https://octave.discourse.group/uploads/short-url/x9g6sUdfUKstx4s6vDZtFapuP3d.txt), the second and third issues are resolved.  However, issue #1 is still not fixed.

Observation #1 : variables, as determined by 'who', show up in scopes where they should not be.

It may be that Octave should be using std::set rather than std::list when it creates the list of variable names that are visible in the current scope.

Rik <rik5>
Group administrator
Mon 28 Jun 2021 09:46:46 PM UTC, comment #6: 

Thank you for looking into this problem.

FWIW the patch that jwe posted at discourse works for me although I am not sure that it fixes all the issues that you found out.

José Matos <jamatos>
Mon 28 Jun 2021 09:25:53 PM UTC, comment #5: 

I'm adding jwe to the CC list because this behavior is just so strange.

Observation #1 : variables, as determined by 'who', show up in scopes where they should not be.

Observation #2 : variables in nested functions which should inherit their value and type from the enclosing top-level function do not do so.

Observation #3 : The incorrect behavior shows up when the top-level function has an input parameter list, but otherwise seems to do the right thing.

Rik <rik5>
Group administrator
Mon 28 Jun 2021 09:18:03 PM UTC, comment #4: 

This is super strange.  It turns out varargin isn't the issue; ANY variable in the input parameter list for the top-level function triggers the behavior.  Here is my latest test case


function mainfcn3 (x)

  [Ahandle, static, fcn_handle] = checkInput();  # A boolean variable
  output = fcn_handle ()  # executed nested function

  function [Ahandle, static, fcn_handle] = checkInput()


Rik <rik5>
Group administrator
Mon 28 Jun 2021 09:13:36 PM UTC, comment #3: 

Interestingly, having nested functions use varargin seems okay.  It is having the main function use varargin which seems to trigger the problem.  This, for example, works


function mainfcn3 ()

  [Ahandle, static, fcn_handle] = checkInput();  # A boolean variable
  output = fcn_handle ()  # executed nested function

  function [Ahandle, static, fcn_handle] = checkInput(varargin)


BUT, this fails


function mainfcn3 (varargin)

  [Ahandle, static, fcn_handle] = checkInput();  # A boolean variable
  output = fcn_handle ()  # executed nested function

  function [Ahandle, static, fcn_handle] = checkInput()


Rik <rik5>
Group administrator
Mon 28 Jun 2021 09:08:28 PM UTC, comment #2: 

Interestingly, I removed all references to varargin (see uploaded file mainfcn3.m) and then the example works correctly.

This is a small clue that the error is only present when varargin is used.

(file #51615)

Rik <rik5>
Group administrator
Mon 28 Jun 2021 09:04:11 PM UTC, comment #1: 

Confirmed.  Thank you for the minimal example.  I added a keyboard statement at the start of nestfcn().  I then executed 'who' to see what variables were available.  The results are odd.


Variables visible from the current scope:

Ahandle     Ahandle     fcn_handle  static      varargin    Ahandle     fcn_handle  static      varargin


As you can see, the variable "Ahandle" is repeated 3 times while other variables are still repeated twice.  Within a given scope, I should see only one instance of a variable so something is wrong.

I then tried 'whos' which maybe helps sort things out a bit.


Variables visible from the current scope:

variables in scope: nestfcn: /home/rik/wip/Projects_Mine/octave-dev/mainfcn2.m

   Attr Name         Size                     Bytes  Class
   ==== ====         ====                     =====  =====
        Ahandle      1x1                          0  function_handle

Total is 1 element using 0 bytes

variables in scope: checkInput: /home/rik/wip/Projects_Mine/octave-dev/mainfcn2.m

   Attr Name            Size                     Bytes  Class
   ==== ====            ====                     =====  =====
    f   Ahandle         1x1                          1  logical
    f   fcn_handle      1x1                          0  function_handle
    f   static          1x1                          1  logical
    f   varargin        0x0                          0  cell

Total is 3 elements using 2 bytes

variables in scope: mainfcn2: /home/rik/wip/Projects_Mine/octave-dev/mainfcn2.m

   Attr Name            Size                     Bytes  Class
   ==== ====            ====                     =====  =====
        Ahandle         1x1                          1  logical
        fcn_handle      1x1                          0  function_handle
        static          1x1                          1  logical
    f   varargin        0x0                          0  cell

Total is 3 elements using 2 bytes


Apparently, with just 'who' I am seeing variables for all scopes, regardless of whether they are really available or not.  Given that I am in the function nestfcn I don't know why I should see variables for the checkInput() scope.  I verified with dbstack where I was


dbstack
stopped in:

  --> mainfcn2>nestfcn at line 15 [/home/rik/wip/Projects_Mine/octave-dev/mainfcn2.m]
              mainfcn2 at line 4 [/home/rik/wip/Projects_Mine/octave-dev/mainfcn2.m]




(file #51614)

Rik <rik5>
Group administrator
Mon 28 Jun 2021 07:54:30 PM UTC, original submission:  

After some discussion in the maintainers channel of discourse I arrived to this function:


function mainfcn2 (varargin)

  [Ahandle, static, fcn_handle] = checkInput(varargin{:});  # A boolean variable
  output = fcn_handle ()  # executed nested function

  function [Ahandle, static, fcn_handle] = checkInput(varargin)
    %% initialize

    Ahandle = true;
    static = false;
    fcn_handle = @nestfcn;  # get a handle to nested function
  end

  function r = nestfcn ()
    if (islogical(Ahandle))
      disp("Success.")
    else
      fprintf("Something went wrong: Ahandle is a %s\n", class(Ahandle))
    end
    r = 1;
  end
end


Calling this function we get:

>> mainfcn2
Something went wrong: Ahandle is a function_handle
output = 1


If the scope was correct I would expect for Ahandle to be a logical value. Instead, in this case, it gets the value of fcn_handle.

The original issue was using the nleigs package but I think that this is a minimal example.

José Matos <jamatos>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #51641:  who-diff.txt added by jwe (841B - text/plain)
file #51615:  mainfcn3.m added by rik5 (532B - text/x-matlab)
file #51614:  mainfcn2.m added by rik5 (559B - text/x-matlab)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2021-07-05 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2021-07-02 jwe Attached File- Added who-diff.txt, #51641
    2021-06-29 jwe StatusConfirmed Ready For Test
    2021-06-28 rik5 SummaryNested functions variable scope when varargin is present Variables in nested functions exhibit multiple problems
        Carbon-Copy- Added jwe
    2021-06-28 rik5 Attached File- Added mainfcn3.m, #51615
        SummaryNested functions variable scope Nested functions variable scope when varargin is present
    2021-06-28 rik5 Attached File- Added mainfcn2.m, #51614
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code