bugGNU Octave - Bugs: bug #61789, new interpreter error for too many...

 
 

bug #61789: new interpreter error for too many input arguments has no error-id (for too few arguments it has one)

Submitter:  Hartmut <hardy>
Submitted:  Mon 10 Jan 2022 09:53:26 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 7.0.90 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 13 Jan 2022 06:27:14 PM UTC, comment #8: 

I don't think I've ever seen any tests checking for more than <Invalid call>, so a more specific message starting similarly might remove most of the need to change BISTs and be fairly simple change.

Nicholas Jankowski <nrjank>
Group Member
Thu 13 Jan 2022 05:56:06 PM UTC, comment #7: 

I think the extra specificity of the error message is important to keep.  If the error message is made vaguely compatible then it might be something like "Invalid call to function (too many input arguments)".  Calling print_usage() is just not very clear in this case because the programmer then has to decipher whether they called the function with the wrong inputs (char vs. double for example) or whether they have made some other error (like too many inputs).

At least for Octave core, we have already removed all BIST tests that tested for extra arguments because that functionality is done by the interpreter.

I don't think it is much of a maintenance burden to push for these same BIST tests to also be removed from packages and user code.  Mostly I think this because most coders don't write BIST tests so there shouldn't be much code out there to check.

Rik <rik5>
Group administrator
Thu 13 Jan 2022 05:45:29 PM UTC, comment #6: 

I agree that it would be better to fix Octave than for everyone to have to update tests.  Is there a reason that we can't just call print_usage for this error?  Or, if print_usage won't work in this context, couldn't we just use the same error message format?

John W. Eaton <jwe>
Group administrator
Thu 13 Jan 2022 05:32:31 PM UTC, comment #5: 

how many BISTs are out there (for non-varargin-using functions) that are still expecting an Invalid call message? e.g.,

%!error <Invalid call> myfunc(1,2,3,4,5,'toomany')

is it easier to make sure the error starts with the same text as print_usage?

Nicholas Jankowski <nrjank>
Group Member
Thu 13 Jan 2022 04:37:03 PM UTC, comment #4: 

I have recompiled Octave's stable branch, including this patch. Now this behavior looks much better.

In the last case, "myfunc(1,2,3)" with too many input arguments, Octave returns now an error-id as before.

The returned error message is still different in this case to before (it is now "myfunc: function called with too many inputs", whereas it was before "Invalid call to myfunc"). But I think I can adapt the image package to this new behavior with relativly small effort.

Thanks for fixing this.


Hartmut <hardy>
Wed 12 Jan 2022 10:29:15 PM UTC, comment #3: 

I pushed the change to stable here: http://hg.savannah.gnu.org/hgweb/octave/rev/064a8fbf9162.

Marking as fixed and closing report.

Rik <rik5>
Group administrator
Wed 12 Jan 2022 05:43:51 PM UTC, comment #2: 

Looks good to me.

Markus Mützel <mmuetzel>
Group administrator
Tue 11 Jan 2022 09:01:00 PM UTC, comment #1: 

I think this is a good idea.  Attached is a cset that does that.  Marking as Patch Submitted.

(file #52648)

Rik <rik5>
Group administrator
Mon 10 Jan 2022 09:53:26 PM UTC, original submission:  

I realised that the interpreter in Octave 7.0.90 automatically throws an error, when a user-defined function is called with too little or too many arguments. This is also properly noted in the NEWS file. And in general I like this improvement of the interpreter.

But this new behavior seems inconsistent to me. I often have functions like this (in the image packge), which should work equally well under Octave 7 as under some older Octave versions.


function result = myfunc(a, b);
  if (nargin != 2)
    print_usage ();
  endif
  disp('I do something');
endfunction


With Octave 6.3.0 (and older Octave versions) I get these error messages:

  • myfunc() with too little input arguments gives
    • error message:   Invalid call to myfunc. ...
    • error-id:        Octave:invalid-fun-call


  • myfunc(1,2,3) with too many input arguments gives
    • error message:   Invalid call to myfunc. ...
    • error-id:        Octave:invalid-func-cal


This seemed consistent to me. And there are many BISTs in place to check that this works properly.

But with the new Octave 7.0.90 I now get this different behavior:

  • myfunc() with too little input arguments gives
    • error message:   Invalid call to myfunc. ...
    • error-id:        Octave:invalid-fun-call
    • This is the same as with older Octave versions.


  • myfunc(1,2,3) with too many input arguments gives
    • error message:    error: myfunc: function falled with too many inputs
    • error-id:         (this is empty)


Especially the last behavior (i.e. the error-id being empty when calling a function with too many input arguments) troubles me. This seems inconsistent to me.

Is this behavior intentional? Or would it be OK to also assigne the same error-id to the last error message (with too many input arguments). This change would make these error messages more consistent and would make it much easier for packages to maintain their code with error message BISTs that work in several versions of Octave.

What do you think?

Hartmut <hardy>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #52648:  errid.cset added by rik5 (1KiB - 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 nrjank (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by hardy (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-01-12 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2022-01-11 rik5 Attached File- Added errid.cset, #52648
        Item GroupNone Regression
        StatusNone Patch Submitted

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code