bugGNU Octave - Bugs: bug #41651, Nested uses of evalin( 'caller',...

 
 

bug #41651: Nested uses of evalin( 'caller', ... )

Submitter:  Michael C. Grant <mcgrant>
Submitted:  Fri 21 Feb 2014 01:58:18 AM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Michael C. Grant Open/Closed:  * Closed
Release:  * dev Operating System:  * Mac OS
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 21 Feb 2014 08:18:13 PM UTC, comment #5: 

I applied your changset.

Thanks.

John W. Eaton <jwe>
Group administrator
Fri 21 Feb 2014 02:08:37 PM UTC, comment #4: 

Patch created. I'm making the assumption that cs[0].scope, which was equal to 1 in all my testing, is unique to the top level of the call stack---so that my shorter fix below is valid. evalin.diff applies this patch.

(file #30631)

Michael C. Grant <mcgrant>
Fri 21 Feb 2014 01:59:58 PM UTC, comment #3: 

I believe I have identified the problem: line 472 of libinterp/corefcn/toplev.cc, in the function "octave_call_stack::do_goto_caller_frame":

if (frame == 0 || (f && f->is_user_code ()))

The "frame == 0" test is an attempt to acknowledge that the top-level frame is equivalent to user code in a function. So far, so good.

But now consider what happens if someone performs an evalin('caller') in a function called from the command line. It will create a new call_stack_elt element with the same scope and context as the command line, and with elt.fcn==0 because there is no calling function; but it will not be at the top of the stack, so frame!=0.

Therefore, this test will fail if evalin('caller') is called again, and this frame will not be treated as user code, as it should.

The corrected code, in my view, should be this:

if (frame == 0 || elt.scope == cs[0].scope || (f && f->is_user_code ()))

or perhaps even  more simply,

if (elt.scope == cs[0].scope || (f && f->is_user_code ()))


Michael C. Grant <mcgrant>
Fri 21 Feb 2014 04:09:50 AM UTC, comment #2: 

Yep, it only happens if the caller is the base workspace. Consider a new function, evalin_test3.m:

function evalin_test3
x = 'In workspace for evalin_test3';
evalin_test1

Calling this function yields the same results in MATLAB and Octave.


(file #30627)

Michael C. Grant <mcgrant>
Fri 21 Feb 2014 03:22:45 AM UTC, comment #1: 

I actually think this failure occurs only if the calling workspace is the base workspace. I'll do some tests to confirm this. (Though it does explain why I did not notice this before; I have been doing most of my testing through scripts, not hand-entered models).

Michael C. Grant <mcgrant>
Fri 21 Feb 2014 01:58:18 AM UTC, original submission:  

Consider the functions evalin_test1 and evalin_test2 attached.

First, look at evalin_test2:

function evalin_test2
x = 'In the workspace for evalin_test2';
disp( evalin( 'caller', 'x' ) );

This function works the same in MATLAB and Octave. Here's MATLAB's output:

>> x = 'In the global workspace';
>> evalin_test2;
In the global workspace

Now let's add a layer of nesting with evalin_test1:

function evalin_test1
x = 'In the workspace for evalin_test1';
evalin( 'caller', 'evalin_test2' );

Since evalin_test2 is evaluated in the global workspace, it should still
give the same result. That is what happens with MATLAB:

>> x = 'In the global workspace';
>> evalin_test2;
In the global workspace

But in Octave, the nesting fails:

>> x = 'In the global workspace';
>> evalin_test2
In the global workspace
>> evalin_test1
In the workspace for evalin_test1

I know this probably sounds obscure, but I actually rely on this behavior
somewhat critically in CVX... now I am somewhat relieved that you did not
include my parser patch in 3.8.1, because it looks like it would not have
been sufficient anyway!

Michael C. Grant <mcgrant>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #30631:  evalin.diff added by mcgrant (806B - application/octet-stream)
file #30627:  evalin_test3.m added by mcgrant (73B - application/octet-stream)
file #30625:  evalin_test2.m added by mcgrant (95B - application/octet-stream)
file #30626:  evalin_test1.m added by mcgrant (99B - application/octet-stream)

 

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 mcgrant (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
    2014-02-21 jwe StatusNone Fixed
        Open/ClosedOpen Closed
    2014-02-21 mcgrant Attached File- Added evalin.diff, #30631
    2014-02-21 mcgrant Attached File- Added evalin_test3.m, #30627
    2014-02-21 mcgrant Attached File- Added evalin_test2.m, #30625
        Attached File- Added evalin_test1.m, #30626

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code