bugGNU Octave - Bugs: bug #64858, Don't do static_cast before type...


bug #64858: Don't do static_cast before type check in pt-bytecode.vm.cc

Submitter:  Petter <petter>
Submitted:  Sat 04 Nov 2023 10:37:01 PM UTC
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
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

Sun 05 Nov 2023 01:52:45 PM UTC, comment #5: 

Thanks. I edited the patch before applying it. Pls review https://hg.savannah.gnu.org/hgweb/octave/rev/713c819e8b14

Arun Giridhar <arungiridhar>
Group Member
Sun 05 Nov 2023 10:37:10 AM UTC, comment #4: 

Please, don't forget to add the bug report number and fix the style before you push the change.

Markus Mützel <mmuetzel>
Group administrator
Sun 05 Nov 2023 10:21:38 AM UTC, comment #3: 

Sorry, I had misread the patch and missed the part where the boolean variable was being set to true, so I took it to be a dummy if. On reading it again, yes, it looks good. Sorry for the noise. I'll push it after breakfast unless you want to push it before.

Arun Giridhar <arungiridhar>
Group Member
Sun 05 Nov 2023 10:03:01 AM UTC, comment #2: 

@arungiridhar: Did you mean `dynamic_cast`? `reinterpret_cast` doesn't look right if the object that `ovb` points to can be of a different class.

If ovb always points to a single object of type `octave_cached_value` if `is_defined ()` returns true, then Petter's change looks good to me. (But I might be missing something.)

Minor nit: Octave's style guide is to use a space between the type and `*` for pointers. I.e., `static_cast<octave_cached_value >` instead of `static_cast<octave_cached_value>`.

Markus Mützel <mmuetzel>
Group administrator
Sun 05 Nov 2023 09:18:48 AM UTC, comment #1: 

This doesn't look right. It looks like a dummy if condition just to placate the compiler, but the basic behavior is still there.

Is it possible / desired for static cast to be reinterpret cast?

Arun Giridhar <arungiridhar>
Group Member
Sat 04 Nov 2023 10:37:01 PM UTC, original submission:  

Move static cast to inside if-statement to avoid undefined behaviour warning.

* pt-bytecode-vm.cc: Move it

Petter <petter>


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

Attach Files:

Attached Files
file #55311:  32535.patch added by petter (2KiB - text/plain)


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by arungiridhar (Posted a comment)
  • -email is unavailable- added by petter (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
    2023-12-23 arungiridhar StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2023-11-05 arungiridhar StatusNone Ready For Test
    2023-11-04 petter Attached File- Added 32535.patch, #55311

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code