bugGNU Octave - Bugs: bug #29447, Cannot create @handles to...

 
 

bug #29447: Cannot create @handles to non-existing functions

Submitter:  Judd Storrs <judd>
Submitted:  Mon 05 Apr 2010 09:41:26 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

Tue 05 Mar 2019 11:01:05 PM UTC, comment #20: 

As of the following changeset, I believe lazy evaluation of function handles works as it does in Matlab:

http://hg.savannah.gnu.org/hgweb/octave/rev/8bd9fd99c12a

See also bug #31821.

John W. Eaton <jwe>
Group administrator
Wed 07 Apr 2010 07:02:25 PM UTC, comment #19: 

"...doesn't actually say it is an error to create an invalid handle. It could be saying that the handle won't work. "

Technically, this is true, but note that the manual uses the wording that some condition must hold at a lot of other places, and usually it means that violating the condition leads to an error. Admittedly, sometimes it means just undefined or incorrect behavior, mainly in the cases where the condition can't be reasonably checked.

"However, I don't think that error() is fundamentally superior to warning() for notifying the user of the possible typo."

It depends on what you consider "fundamentally", but I think this is not true. Unlike a warning, error normally produces a traceback to point you to the place where it occured. Warnings can only do this if you turn them into errors.
I should add that apparently many Octave users utterly dislike warnings, so if they ever used niftimatlib, they would probably complain even about the warning.

In any case, as you said, the manual clearly implies that a program creating handle to a function that is not in scope is not valid.

Jaroslav Hajek <highegg>
Wed 07 Apr 2010 05:24:29 PM UTC, comment #18: 

I'm no longer interested in having this fixed in octave, it looks like niftimatlib needs to be looked over carefully for other reasons as well. I have the feeling it has accumulated a few other nasty "tricks" to work around limitations in older Matlab releases and I'm starting to favor a native NIfTI interface perhaps to the reference C-libraries for octave. "Won't fix" is fine with me.

Judd Storrs <judd>
Wed 07 Apr 2010 02:58:07 PM UTC, comment #17: 

"At the time you create a function handle, the function you specify must be on the MATLAB path and in the current scope of the code creating the handle."

...doesn't actually say it is an error to create an invalid handle. It could be saying that the handle won't work.

I think there are two issues that are being discussed here:

  1. Do we allow invalid handles to be created?
  2. Do we allow invalid handles to possibly be repaired when they are used?


With respect to the first, I think the permissiveness of allowing creation of an invalid handle but making it an error to use an invalid handle an error is consistent with the language as a whole.

The criticism that handles can result from typos is a valid concern. However, I don't think that error() is fundamentally superior to warning() for notifying the user of the possible typo. The latest patch I posted issues warnings on creation of invalid handles and errors on use.

Is an error at creation better than a warning? The Matlab debugger can be configured break on warnings. I don't know if it's currently possible to switch the octave interpreter into an mode that treats all warnings as errors. I think that in itself would be useful. But it is a different issue.

[ Just as a philosophical aside, in these interactive numerical languages there was a similar debate about whether or not to allow empty matrices and one argument against having empty matrices is that an empty matrix can never be indexed--so don't even let the user have the option of creating an empty matrix. Anyway, Matlab decided to allow empty matrices and issue an error when indexing the empty matrix, but IDL forbade them instead. ]

With respect to the second point--whether to repair a broken handle, I think the quote from the documentation is not ambiguous in you should probably not expect invalid handles to be repaired.

Judd Storrs <judd>
Wed 07 Apr 2010 08:51:57 AM UTC, comment #16: 

I just discovered the following statement in Matlab's online docs:
"At the time you create a function handle, the function you specify must be on the MATLAB path and in the current scope of the code creating the handle. For example, ..."

That seems pretty clear to me. If the most recent Matlab still allows making a handle to a non-existing function, it is in contradiction with its own docs. This makes it even more likely a bug that is going to be fixed.

Jaroslav Hajek <highegg>
Wed 07 Apr 2010 07:05:52 AM UTC, comment #15: 

I tried that with Matlab 2007a. I opened up the guide, put a button on the form, the right-clicked on the button and selected "Callback". The editor opened up with an m-file containing handler code for the button (and presumably the whole form). If there were any invalid handles, I didn't notice them. Is there something else I can try>

   1. Is it documented anywhere that Matlab does not work this way?

Hardly. However, undocumented behavior like this can easily be changed in the future, especially if the new behavior is better (which it is). If your observation about GUIDE is true, that may be a good reason for MathWorks not to do it, but maybe it's not relying on this anymore.

   2. Is it important that invalid handles throw errors despite a new function appearing in the path?

As I said, it's obviously better (if it can be easily done) to show a bug where it is, not where it first causes problems. "Important" is probably subjective. Is it important to "fix" this?

Jaroslav Hajek <highegg>
Wed 07 Apr 2010 06:53:57 AM UTC, comment #14: 

I haven't done Matlab GUI stuff in at least five years, but that's how I remember it. You create the GUI, open up the parameter panels and enter callback functions--you can enter strings or function handles but they both behaved the same. Then you launch the GUI and click around to watch errors appear in the command window reminding you what needs to be implemented next. Then you create the missing m-files and repressing the buttons works without regenerating or reopening the GUI--so yes the handles certainly seem to figure out that a new file exists.

Let me turn your questions around:

  1. Is it documented anywhere that Matlab does not work this way?
  2. Is it important that invalid handles throw errors despite a new function appearing in the path?


I doubt it because currently invalid handles aren't allowed at all. ;)

Judd Storrs <judd>
Wed 07 Apr 2010 06:18:24 AM UTC, comment #13: 

Does it really create invalid handles first? I sort of had the impression that it creates m-files first. Actually I never really used it, I just tried what it does.
In any case, even if it was true that 99,9% people use GUIDE (which I seriously doubt), this simply doesn't apply to Octave.

Jaroslav Hajek <highegg>
Wed 07 Apr 2010 06:07:00 AM UTC, comment #12: 

I would say that in Matlab 99.9% of invalid handles come into existence as callbacks while using GUIDE to mock up a GUI before/while implementing the backend code. If the handles were broken from creation, the way GUIDE works would be fairly impossible.

Judd Storrs <judd>
Wed 07 Apr 2010 05:47:16 AM UTC, comment #11: 

By the way, is this a documented behavior of Matlab? I couldn't find it anywhere. Matlab docs say that a handle stores an absolute path to a function file, from which I imply that cd to a different directory should make no difference in subsequent calls. So, what is stored if no function can be found?
Btw, which version did you use in your test?

Weird tricks done by libraries are probably < 1% of the occurences of non-existing handles. The remaining 99% are names that were simply misspelled, or missing functions or packages. In these cases, it makes much more sense to throw an error at the point where the handle is created, because that's where the bug actually is. Sometimes this is the same line where the first call occurs, but it may not be.

Maybe the reason why niftimatlib does this is so that you could write your own error handler? In that case, I'd say just do it. Maybe you can convince them to do it directly in the library? After all, it seems that you will get something like "undefined function or variable 'crash'" from Matlab, which is not really an informative error message here.

I feel a bit tired of defending the common sense against Matlab quirks. Go ahead and "fix" this if I didn't convince you. It would be nice if it could be turned off by a global pseudo-variable, however.

Jaroslav Hajek <highegg>
Wed 07 Apr 2010 03:34:39 AM UTC, comment #10: 

Here's a different patch that addresses the criticisms of the first one. Instead of returning a string it returns an invalid handle.

I think the behavior is clearer:


octave:1> func = @crash
warning: @crash: no function and no method found
func = @crash
octave:2> func()
error: crash: no longer valid function handle
octave:2> feval(func)
error: crash: not a valid function handle
octave:2>


Changes:


diff -r 6cf7d7665d57 src/ov-fcn-handle.cc
--- a/src/ov-fcn-handle.cc        Fri Apr 02 13:08:11 2010 -0400
+++ b/src/ov-fcn-handle.cc        Tue Apr 06 23:24:21 2010 -0400
@@ -1523,7 +1523,11 @@
             }
         }
       else
-        error ("@%s: no function and no method found", tnm.c_str ());
+        {
+          warning ("@%s: no function and no method found", tnm.c_str ());
+          octave_fcn_handle *fh = new octave_fcn_handle (f, tnm);
+          retval = fh;
+        }
     }

   return retval;
diff -r 6cf7d7665d57 src/ov-fcn-handle.h
--- a/src/ov-fcn-handle.h        Fri Apr 02 13:08:11 2010 -0400
+++ b/src/ov-fcn-handle.h        Tue Apr 06 23:24:21 2010 -0400
@@ -96,7 +96,14 @@
   dim_vector dims (void) const { static dim_vector dv (1, 1); return dv; }

   octave_function *function_value (bool = false)
-    { return fcn.function_value (); }
+    {
+      octave_function *retval = 0;
+      if (fcn.is_defined ())
+        retval = fcn.function_value ();
+      else
+        error ("%s: not a valid function handle", nm.c_str ());
+      return retval ;
+    }

   octave_user_function *user_function_value (bool = false)
     { return fcn.user_function_value (); }


Judd Storrs <judd>
Wed 07 Apr 2010 03:25:50 AM UTC, comment #9: 

I've figured out how to get an inline function created and substituted (in case anyone other than me was interested in how to do that). I'm actually not a fan of this approach after seeing the results:


diff -r 6cf7d7665d57 src/ov-fcn-handle.cc
--- a/src/ov-fcn-handle.cc        Fri Apr 02 13:08:11 2010 -0400
+++ b/src/ov-fcn-handle.cc        Tue Apr 06 21:31:25 2010 -0400
@@ -1523,7 +1523,20 @@
             }
         }
       else
-        error ("@%s: no function and no method found", tnm.c_str ());
+        {
+          // Failed to resolve the handle
+          // Substitute an inline function that calls error()
+
+          warning ("@%s: no function and no method found", tnm.c_str ());
+
+          std::ostringstream buf;
+          buf << "@()error('@" << tnm << ": no function and no method found')" ;
+
+          int parse_status ;
+          octave_value anon_fcn_handle = eval_string (buf.str (), true, parse_status);
+
+          retval = anon_fcn_handle;
+        }
     }

   return retval;


This is how it behaves:


octave:1> func = @crash
warning: @crash: no function and no method found
func =

@() error ('@crash: no function and no method found')

octave:2> func()
error: @crash: no function and no method found
error: called from:
error:    at line -1, column -1
octave:2> feval(func)
error: @crash: no function and no method found
error: called from:
error:    at line -1, column -1
octave:2>


Judd Storrs <judd>
Tue 06 Apr 2010 07:01:31 PM UTC, comment #8: 

The invalid handle is explicitly "@crash" and is intended to crash on evaluation. It's used in a data structure that stores data type information related to the NIfTI file format. Specifically, @crash is used to mark currently unsupported datatypes (FLOAT128 and COMPLEX256). For example, in @file_array/private/datatypes.m:


    t = true;
    f = false;
    table = {...
        0   ,'UNKNOWN'   ,'uint8'   ,@uint8  ,1,1  ,t,t,f
        1   ,'BINARY'    ,'uint1'   ,@logical,1,1/8,t,t,f
        256 ,'INT8'      ,'int8'    ,@int8   ,1,1  ,t,f,t
        2   ,'UINT8'     ,'uint8'   ,@uint8  ,1,1  ,t,t,t
        4   ,'INT16'     ,'int16'   ,@int16  ,1,2  ,t,f,t
        512 ,'UINT16'    ,'uint16'  ,@uint16 ,1,2  ,t,t,t
        8   ,'INT32'     ,'int32'   ,@int32  ,1,4  ,t,f,t
        768 ,'UINT32'    ,'uint32'  ,@uint32 ,1,4  ,t,t,t
        1024,'INT64'     ,'int64'   ,@int64  ,1,8  ,t,f,f
        1280,'UINT64'    ,'uint64'  ,@uint64 ,1,8  ,t,t,f
        16  ,'FLOAT32'   ,'float32' ,@single ,1,4  ,f,f,t
        64  ,'FLOAT64'   ,'double'  ,@double ,1,8  ,f,f,t
        1536,'FLOAT128'  ,'float128',@crash  ,1,16 ,f,f,f
        32  ,'COMPLEX64' ,'float32' ,@single ,2,4  ,f,f,f
        1792,'COMPLEX128','double'  ,@double ,2,8  ,f,f,f
        2048,'COMPLEX256','float128',@crash  ,2,16 ,f,f,f
        128 ,'RGB24'     ,'uint8'   ,@uint8  ,3,1  ,t,t,f};


Probably niftimatlib should include a crash.m if they want to be certain that their @crash will actually crash ;)

The mechanisms at work in the following may be usable for building a patch if we wanted to switch the behavior from fail-on-creation to fail-on-use.


octave:1> type crash
crash is the user-defined function defined from: /tmp/test2/crash.m

function crash()
error("Crash!!")
endfunction

octave:2> func = @crash
func = @crash
octave:3> ^Z
[2]+  Stopped                 octave
$ rm crash.m
$ fg
octave:3> func
func = @crash
octave:4> func()
error: crash: no longer valid function handle


Judd Storrs <judd>
Tue 06 Apr 2010 05:58:17 PM UTC, comment #7: 

OK, looking at this some more, I don't think what I proposed is at all the right thing for Matlab compatibility.  Apparently you can do

  f = @nofun;

for a function that is currently not visible anywhere in Matlab's path, then you can cd to a directory that contains a function called nofun, or evaluate f in some other context where a function (or subfunction) called nofun exists, and that function will be called.

So we really do need something different here if we want compatibility.  It seems that we need to be able to create the function handle object but without doing the function lookup in advance, then later perform the lookup if the handle is not already defined.

Exactly how and why is this used in niftimatlib?  Is it to set up a list of function handles that are later evaluated in different contexts with different expected results (sometimes a  function is found, other times not)?  Or is it something else?

John W. Eaton <jwe>
Group administrator
Tue 06 Apr 2010 05:04:22 PM UTC, comment #6: 

I don't know if it's possible but I think that creating the anonymous function would be the preferred solution.

I started to look at how to do that before the string idea and I haven't yet figured out how that works. From what I currently understand it seems that anonymous function creation happens inside the parser but handle creation happens slightly later. I'm suspecting that the anonymous function creation requires scopes and other parser states that don't seem immediately available. Anyway, I don't understand that part of octave at this point (but would like to) so I'll look at it further.

Judd Storrs <judd>
Tue 06 Apr 2010 05:01:18 PM UTC, comment #5: 

I agree that returning a string is a bad idea as you want be able to call the function without going through 'feval'. That is, the following will not work

  f = @foo;
  f (7)

(it will try to index the string).

How about just returning an anonymous function that calls 'feval'? Something like

  f = @(varargin) feval ("foo", varargin {:})

should then be returned when the user constructs '@foo'.

Søren Hauberg <hauberg>
Tue 06 Apr 2010 04:52:28 PM UTC, comment #4: 

It seems to me that returning a string with the name of the function in this case is not the right thing to do, because at the point where the handle is used (in your feval call) there could be a function with that name that is visible that was not visible when the function handle was created.  So it seems it would be better to create an anonymous function like

  msg = sprintf ("no function or method `%s' in this context",
                 function_name);
  retval = @() error (msg);

or similar.  Of course, this needs to be done in C++.

Is there any reason that we can't do what I'm proposing above instead of just throwing the error immediately when the function handle is created?

John W. Eaton <jwe>
Group administrator
Tue 06 Apr 2010 04:29:14 PM UTC, comment #3: 

Here's a slightly different idea--if the handle is invalid print a warning and return the function name as a string instead.


diff -r 6cf7d7665d57 src/ov-fcn-handle.cc
--- a/src/ov-fcn-handle.cc        Fri Apr 02 13:08:11 2010 -0400
+++ b/src/ov-fcn-handle.cc        Tue Apr 06 12:23:50 2010 -0400
@@ -1523,7 +1523,10 @@
             }
         }
       else
-        error ("@%s: no function and no method found", tnm.c_str ());
+        {
+          warning ("@%s: no function and no method found", tnm.c_str ());
+          retval = tnm;
+        }
     }

   return retval;


I'm not sure that I did the octave_value string creation correctly or if I need to add something for memory management. "make check" is the same with and without the change (all pass except for one eigs failure).

This is how octave behaves with the change, which seems reasonable to me:


octave:1> func = @crash
warning: @crash: no function and no method found
func = crash
octave:2> typeinfo func
ans = sq_string
octave:3> feval(func)
error: feval: function `crash' not found
octave:3>


If this approach is acceptable I can create a changeset.

Judd Storrs <judd>
Tue 06 Apr 2010 02:35:40 PM UTC, comment #2: 

It seems to me that it should be possible for octave to create and return the lambda automatically instead of requiring the m-files to be rewritten? I'll look into this. This is not a show stopper for me at the moment and I'll muddle with this.

The particular example was provided to demonstrate that this feature is "used in the wild" by a team of very experienced Matlab coders. niftimatlib is a small data i/o portion of the much larger GPL-licensed SPM http://www.fil.ion.ucl.ac.uk/spm/ -- one of the premier packages for neuroimaging analysis and very heavily Matlab-based. SPM makes heavy use of the object type system. Anyway, very little of SPM actually works in octave at this point and I view niftimatlib as widely used pre-extracted fragment that can help chip away at some of the incompatibilities and discrepancies between the interpreters.

Judd Storrs <judd>
Tue 06 Apr 2010 06:17:09 AM UTC, comment #1: 

The situation you describe is really special and I can imagine a better solution on the library's side, such as returning an anonymous or private function handle containing an error() call.

Other than this weirdness, are there any good reasons why defer the error to first call? If you misspell a function name, I think it makes a much better sense to gripe at the point of handle creation, instead of the point of usage, which can be nowhere near the bug. Besides, with Octave's current implementation, griping at creation is more natural and more efficient.

I think Octave's behavior is better and I'm against such a change unless you can bring up better arguments.

Jaroslav Hajek <highegg>
Mon 05 Apr 2010 09:41:26 PM UTC, original submission:  

Octave fails to create function handles to non-existing functions.

octave-3.3.51+:1> func = @crash
error: @crash: no function and no method found

Matlab does allows creation of the handle, but crashes on use instead:

>> func = @crash

func =
    @crash

>> feval(func)

??? Error using ==> feval
Undefined function or variable 'crash'.

Crashing on use is actually the desired behavior. My suggestion is that octave creates an invalid handle in these cases that fails on evaluation.

Background:

niftimatlib (niftilib.sf.net) uses a function lookup matrix for dispatch while reading NIfTI files. The lookup is based on a combination of header fields (only certain combinations are allowed and/or plausible and/or sane and/or implemented). The invalid entries are marked with @crash_me. The intent is that Matlab crashes on invalid input.

Workaround:

Create crash_me.m containing an error() call.

Judd Storrs <judd>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2019-03-06 rik5 StatusWont Fix Fixed
    2010-04-07 jwe StatusNeed Info Wont Fix
        Open/ClosedOpen Closed
    2010-04-06 highegg StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code