[Octave-patch-tracker] [patch #10139] interpreter: static instead of dynamic casts for speed

[Markus Mützel]

Update of patch #10139 (project octave):

                  Status:                    None => In Progress            

    _______________________________________________________

Follow-up Comment #1:

Sorry for taking so long for coming around to this.

This is definitely interesting. I'm not 100% sure if a static_cast will always
be safe. But that seems to be the case afaict (at least for Octave's own
codebase).
And I like the approach of optionally turning the "dynamic_cast" back on.

Just a minor comment: Should the new macro be renamed from DYNORSTAT_CAST to
something like OCTAVE_OBV_CAST to avoid potential clashes of the macro name
when the `ops.h` header is included in downstream projects?
IIUC, these casts are all between types that are derived from
`octave_base_value`. Hence, the "OBV" in the proposed macro name.
But other names are also ok as long as they start with "OCTAVE_" - I guess.

Also: Should the configure switch apply to downstream projects that include
`ops.h`? In that case, OCTAVE_ENABLE_INTERNAL_CHECKS should probably also be
included in `oct-conf-post-public.in.h`.
Alternatively, we could default to `dynamic_cast` for user code if we are
uncertain what could happen downstream. Perhaps with the option to explicitly
switch to `static_cast` by passing `-DOCTAVE_NO_INTERNAL_CHECKS` or something
similar...

I'm attaching an updated patch that applies on the current default branch.


(file #52986)
    _______________________________________________________

Additional Item Attachment:

File name: patch10139-staticcast_tomner-v2.patch Size:230 KB
   
<https://file.savannah.gnu.org/file/patch10139-staticcast_tomner-v2.patch?file_id=52986>



    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?10139>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/