bugGNU Octave - Bugs: bug #65538, xint_value does not work the way...

 
 

bug #65538: xint_value does not work the way it is intended

Submitter:  Arun Giridhar <arungiridhar>
Submitted:  Fri 29 Mar 2024 08:19:49 PM UTC
   
 
Category:  Coding Style and Maintenance Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Missed Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  10.1.0 (current default) Planned Release:  10.1.0 (current default)
* Mandatory Fields

Add a New Comment Rich Markup
   

Sun 14 Apr 2024 04:36:25 PM UTC, comment #5: 

Looks stable. Closing as fixed.

Arun Giridhar <arungiridhar>
Group Member
Tue 09 Apr 2024 01:53:14 AM UTC, comment #4: 

I pushed an edited version of this patch after combining and tightening up the new function definitions.

https://hg.savannah.gnu.org/hgweb/octave/rev/fb44a9db1bc6

Works as intended so far in tests. Marking as ready for test.

Do we need to add any new BISTs?

Arun Giridhar <arungiridhar>
Group Member
Mon 01 Apr 2024 01:54:24 AM UTC, comment #3: 

Linking related discussion about this bug report:
https://octave.discourse.group/t/inconsistent-conversion-to-integers/5438

Arun Giridhar <arungiridhar>
Group Member
Mon 01 Apr 2024 01:46:46 AM UTC, comment #2: 

I've attached two patches to introduce a few new functions in ov.h and ov.cc, and to use them elsewhere.

I've retargeted to default instead of stable since the API changes with the addition of new functions.

Without these two patches, in addition to the problems described in comment #0, code like `cell (e, pi)` silently creates a 2x3 cell array, unlike functions like `zeros` and `ones` which at least complain about non-integer dimensions.

With the patches, those missed errors are fixed. Non-boolean values being converted to boolean function inputs now give a warning.

When testing, apply integer.patch first, then the idx_bool.patch next.

(file #55909, file #55910)

Arun Giridhar <arungiridhar>
Group Member
Fri 29 Mar 2024 09:54:46 PM UTC, comment #1: 

Maybe this should be on Discourse, rather than the bug tracker, but one answer would be to use int_value() with the first input set to true.  The prototype for the function in ov.h is


  int int_value (bool req_int = false, bool frc_str_conv = false) const


The first input, require_integer, is defaulted to false.  This will emit an error about not having an integer input, but the message is not customizable.  For that, you would need to put the call to int_value() in a try/catch block.

xint_value() only emits an error when it can't convert the input to an integer, not when the original value is not something that can be coerced to integer.  For example,


all (1, {1})
error: all: DIM must be an integer


Rik <rik5>
Group administrator
Fri 29 Mar 2024 08:19:49 PM UTC, original submission:  

There are several instances of xint_value in the codebase like this in the all() DEFUN in data.cc:


  int dim = (nargin == 1 ? -1
             : args(1).xint_value ("all: DIM must be an integer")-1);


This looks like it will trap non-integer inputs to DIM, but nothing happens:


octave:1> all (true(3,3), pi)
ans =
  1  1  1
  1  1  1
  1  1  1

octave:2> all (true(3,3), e)
ans =
  1
  1
  1

octave:3> all (true(3,3), (sqrt(5) + 1) / 2)
ans =
  1  1  1


What is the correct way to trap non-integer input?

Arun Giridhar <arungiridhar>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #55909:  integer.patch added by arungiridhar (14KiB - text/x-patch)
file #55910:  idx_bool.patch added by arungiridhar (14KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by arungiridhar (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-04-14 arungiridhar StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 10.1.0 (current default)
    2024-04-09 arungiridhar StatusPatch Submitted Ready For Test
    2024-04-01 arungiridhar Attached File- Added integer.patch, #55909
        Attached File- Added idx_bool.patch, #55910
        StatusNone Patch Submitted
        Planned Release9.2.0 10.1.0 (current default)

    Back to the top

    Powered by Savane 3.13-3e34.
    Corresponding source code