octave-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-bug-tracker] [bug #61753] Changing assert to panic_impossible


From: Rik
Subject: [Octave-bug-tracker] [bug #61753] Changing assert to panic_impossible
Date: Tue, 4 Jan 2022 19:52:36 -0500 (EST)
User-agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/95.0.4638.69 Safari/537.36

Follow-up Comment #11, bug #61753 (project octave):

I think this project is slightly more involved than just replacing assert with
panic_unless.  The goal is actually to remove as many land mines from the code
as possible which means replacing assert with calls to error() where possible
and only as a last resort doing a call to panic_unless() or panic_impossible.

I believe one key to distinguishing between the two cases is whether the code
maintains state.  For example, the parser maintains state about where it is in
an input stream and what token it is processing.  If it encounters a condition
that it believes is impossible it certainly can't go forward and continue
processing, nor can it go backwards and just return to the Octave prompt
because now the interpreter will be lost as to where it is in the input
stream.  In this case executing an abort() seems like the only way out.

On the other hand, if Octave is just performing a calculation (which therefore
doesn't have any state) and it runs into trouble it can just stop the
calculation, discard any calculated results, and return to the Octave prompt.

As an example, the first change in the diff you made is


diff -r 2f1fae9dd79d libinterp/corefcn/cellfun.cc
--- a/libinterp/corefcn/cellfun.cc      Tue Jan 04 08:58:31 2022 +0100
+++ b/libinterp/corefcn/cellfun.cc      Tue Jan 04 18:10:41 2022 -0500
@@ -1990,8 +1990,8 @@
 do_mat2cell_2d (const Array2D& a, const Array<octave_idx_type> *d, int nd)
 {
   Cell retval;
-  assert (nd == 1 || nd == 2);
-  assert (a.ndims () == 2);
+  panic_unless (nd == 1 || nd == 2);
+  panic_unless (a.ndims () == 2);
 
   if (mat2cell_mismatch (a.dims (), d, nd))
     return retval;


do_mat2cell_2d() is an internal function that is eventually called when a user
executes mat2cell().  There is no reason this sort of input validation
couldn't be replaced with calls to error() which simply send the user back to
the Octave prompt.

In this case, even error() is unnecessary.  A search through the code reveals
do_mat2cell_2d is called from only 3 places and they all have code that
guarantees 2-D matrices.  For example, the dispatch for do_mat2cell is


template <typename ArrayND>
Cell
do_mat2cell (const ArrayND& a, const Array<octave_idx_type> *d, int nd)
{
  if (a.ndims () == 2 && nd <= 2)
    return do_mat2cell_2d (a, d, nd);


which already checks the necessary conditions.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?61753>

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]