Refactor C++ code to eliminate useless return statements after error()

From Octave
Revision as of 07:16, 12 December 2015 by Andy1978 (talk | contribs)
Jump to navigation Jump to search

Introduction

The C++ error handling mechanism in core Octave has been changed to use exceptions. Previously, calling error() in C++ would always return execution to the calling function. This made it necessary to explicitly use the return keyword to exit a function, or arrange for an if/else tree in the original function so that normal function execution would not continue in the case of an error. With exceptions, the C++ code may now be written in a manner that closely resembles Octave's own m-file language.

The principal work for this sprint topic is to remove the unnecessary return statements which follow many calls to error ().

Example 1 : Eliminate return (F_DUPFD() from syscalls.cc)

Before After
#if defined (F_DUPFD)
  return const_value (args, F_DUPFD);
#else
  error ("F_DUPFD: not available on this system");
  return octave_value ();
#endif
#if defined (F_DUPFD)
  return const_value (args, F_DUPFD);
#else
  error ("F_DUPFD: not available on this system");
#endif

Example 2 : Eliminate retval and code block { } (make_idx_args() from ov-base.cc)

Before After
if (val.is_string ())
  subs_field(i) = val;
else
  {
    error ("string argument required for '.' index");
    return retval;
  }
if (val.is_string ())
  subs_field(i) = val;
else
  error ("string argument required for '.' index");

Example 3 : Elimination and input validation first (graphics.in.h)

Before After
virtual graphics_handle get_parent (void) const
{
  if (valid_object ())
    return get_properties ().get_parent ();
  else
    {
      error ("base_graphics_object::get_parent: invalid graphics object");
      return graphics_handle ();
    }
}
virtual graphics_handle get_parent (void) const
{
  if (! valid_object ())
    error ("base_graphics_object::get_parent: invalid graphics object");

  return get_properties ().get_parent ();
}

Detailed Instructions

The list of files which contain an instance of error() followed immediately by a return statement is shown in the Files section of this page. The actual instances, including line numbers, are shown in the Instances section. To avoid duplication, sign up for a particular file by editing the Files section of this wiki page and replacing '???' with your name. When you have edited a file you should verify that everything is okay by executing

make all
./run-octave
test FILENAME

When that passes, let a Maintainer know so that we can check in the changes. Also, add the wiki tags

<strike> ... </strike>

to the Files section to cross the file off the list. In addition, increment the number of files that were fixed by +1.

Files

Start of Sprint

Total: 53

Fixed: 0

Owner File
??? corefcn/data.cc
??? corefcn/graphics.cc
??? corefcn/graphics.in.h
??? corefcn/ls-mat4.cc
??? corefcn/ls-mat5.cc
??? corefcn/oct-fstrm.cc
??? corefcn/oct-stream.cc
??? corefcn/oct-strstrm.cc
??? corefcn/syscalls.cc
??? dldfcn/__delaunayn__.cc
Andy dldfcn/__fltk_uigetfile__.cc
Andy dldfcn/__init_fltk__.cc
??? octave-value/ov-base-int.cc
??? octave-value/ov-base.cc
??? octave-value/ov-bool-sparse.cc
??? octave-value/ov-bool.cc
??? octave-value/ov-cell.cc
??? octave-value/ov-class.cc
??? octave-value/ov-classdef.h
??? octave-value/ov-complex.cc
??? octave-value/ov-cx-sparse.cc
??? octave-value/ov-fcn-inline.cc
??? octave-value/ov-float.cc
??? octave-value/ov-flt-complex.cc
??? octave-value/ov-java.cc
??? octave-value/ov-range.cc
??? octave-value/ov-re-sparse.cc
??? octave-value/ov-scalar.cc
??? octave-value/ov-struct.cc
??? octave-value/ov-usr-fcn.cc
??? octave-value/ov.cc
??? operators/op-cm-cm.cc
??? operators/op-cm-m.cc
??? operators/op-cm-scm.cc
??? operators/op-cm-sm.cc
??? operators/op-fcm-fcm.cc
??? operators/op-fcm-fm.cc
??? operators/op-fm-fcm.cc
??? operators/op-fm-fm.cc
??? operators/op-int.h
??? operators/op-m-cm.cc
??? operators/op-m-m.cc
??? operators/op-m-scm.cc
??? operators/op-m-sm.cc
??? operators/op-scm-cm.cc
??? operators/op-scm-m.cc
??? operators/op-scm-scm.cc
??? operators/op-scm-sm.cc
??? operators/op-sm-cm.cc
??? operators/op-sm-m.cc
??? operators/op-sm-scm.cc
??? operators/op-sm-sm.cc
??? parse-tree/pt-exp.cc

Instances

Start of Sprint

Total: 127

Fixed: 0

corefcn/graphics.in.h:70
corefcn/graphics.in.h:76
corefcn/graphics.in.h:82
corefcn/graphics.in.h:88
corefcn/graphics.in.h:319
corefcn/graphics.in.h:326
corefcn/graphics.in.h:332
corefcn/graphics.in.h:407
corefcn/graphics.in.h:2739
corefcn/graphics.in.h:2750
corefcn/graphics.in.h:2761
corefcn/graphics.in.h:2774
corefcn/graphics.in.h:2780
corefcn/graphics.in.h:2801
corefcn/graphics.in.h:2812
corefcn/graphics.in.h:2855
corefcn/graphics.in.h:2862
corefcn/graphics.in.h:2892
octave-value/ov-classdef.h:420
octave-value/ov-classdef.h:550
octave-value/ov-classdef.h:1518
octave-value/ov-classdef.h:1532
operators/op-int.h:633
operators/op-int.h:655
corefcn/data.cc:2071
corefcn/data.cc:5370
corefcn/data.cc:6749
corefcn/data.cc:7695
corefcn/data.cc:7750
corefcn/data.cc:7762
corefcn/graphics.cc:3002
corefcn/graphics.cc:11658
corefcn/ls-mat4.cc:404
corefcn/ls-mat5.cc:1470
corefcn/ls-mat5.cc:2688
corefcn/oct-fstrm.cc:68
corefcn/oct-fstrm.cc:77
corefcn/oct-stream.cc:2612
corefcn/oct-strstrm.cc:35
corefcn/oct-strstrm.cc:44
corefcn/syscalls.cc:1596
corefcn/syscalls.cc:1612
corefcn/syscalls.cc:1628
corefcn/syscalls.cc:1644
corefcn/syscalls.cc:1660
corefcn/syscalls.cc:1677
corefcn/syscalls.cc:1693
corefcn/syscalls.cc:1710
corefcn/syscalls.cc:1726
corefcn/syscalls.cc:1743
corefcn/syscalls.cc:1759
corefcn/syscalls.cc:1776
corefcn/syscalls.cc:1792
corefcn/syscalls.cc:1809
corefcn/syscalls.cc:1825
dldfcn/__delaunayn__.cc:79
<strike>dldfcn/__fltk_uigetfile__.cc:144</strike>
<strike>dldfcn/__init_fltk__.cc:2085</strike>
<strike>dldfcn/__init_fltk__.cc:2108</strike>
<strike>dldfcn/__init_fltk__.cc:2123</strike>
octave-value/ov-base-int.cc:514
octave-value/ov-base.cc:131
octave-value/ov-base.cc:163
octave-value/ov-base.cc:172
octave-value/ov-base.cc:199
octave-value/ov-base.cc:207
octave-value/ov-base.cc:1289
octave-value/ov-base.cc:1347
octave-value/ov-base.cc:1626
octave-value/ov-base.cc:1632
octave-value/ov-bool-sparse.cc:281
octave-value/ov-bool.cc:135
octave-value/ov-cell.cc:244
octave-value/ov-cell.cc:549
octave-value/ov-class.cc:281
octave-value/ov-class.cc:2089
octave-value/ov-class.cc:2130
octave-value/ov-complex.cc:317
octave-value/ov-cx-sparse.cc:313
octave-value/ov-fcn-inline.cc:785
octave-value/ov-fcn-inline.cc:806
octave-value/ov-float.cc:153
octave-value/ov-flt-complex.cc:261
octave-value/ov-java.cc:2092
octave-value/ov-java.cc:2461
octave-value/ov-java.cc:2484
octave-value/ov-range.cc:478
octave-value/ov-re-sparse.cc:344
octave-value/ov-scalar.cc:168
octave-value/ov-struct.cc:285
octave-value/ov-struct.cc:1173
octave-value/ov-struct.cc:1760
octave-value/ov-struct.cc:2014
octave-value/ov-struct.cc:2020
octave-value/ov-struct.cc:2037
octave-value/ov-struct.cc:2044
octave-value/ov-struct.cc:2052
octave-value/ov-usr-fcn.cc:122
octave-value/ov-usr-fcn.cc:510
octave-value/ov-usr-fcn.cc:846
octave-value/ov-usr-fcn.cc:957
octave-value/ov-usr-fcn.cc:1053
octave-value/ov.cc:2975
octave-value/ov.cc:2998
operators/op-cm-cm.cc:92
operators/op-cm-m.cc:76
operators/op-cm-scm.cc:78
operators/op-cm-sm.cc:77
operators/op-fcm-fcm.cc:93
operators/op-fcm-fm.cc:82
operators/op-fm-fcm.cc:81
operators/op-fm-fm.cc:80
operators/op-m-cm.cc:77
operators/op-m-m.cc:79
operators/op-m-scm.cc:78
operators/op-m-sm.cc:76
operators/op-scm-cm.cc:65
operators/op-scm-m.cc:66
operators/op-scm-scm.cc:126
operators/op-scm-sm.cc:76
operators/op-sm-cm.cc:65
operators/op-sm-m.cc:63
operators/op-sm-scm.cc:76
operators/op-sm-sm.cc:100
parse-tree/pt-exp.cc:58
parse-tree/pt-exp.cc:65
parse-tree/pt-exp.cc:78