Re: purpose of 10486:4e64fbbd5c58

[John W. Eaton]

On  6-Apr-2010, Jaroslav Hajek wrote:

| I guess it would take a not-quite-trivial effort to make it exactly
| Matlab compatible (cf. the above example). If you don't make it Matlab
| compatible, you sort of lose the primary motivation for this whole
| thing.

I think for the purposes of the person I am working with, it would be
OK if Octave allowed both

  x(1:1.7:9)

and

  i = 1:1.7:9;
  x(i)

and it would also be OK if this is only enabled with --traditional.

| Do as you wish if this didn't convince you, I think I've exhausted all
| my arguments. I know you have a negative attitude towards more global
| variables, but if it's possible, I'd like to have a way to turn this
| stupidity off.

If we use a warning with an ID, then couldn't you just turn the
specific warning into an error?

Is the following patch OK with you?  It's based on the changeset you
sent earlier.  With it, I see

  octave:1> x = 1:10
  x =

      1    2    3    4    5    6    7    8    9   10

  octave:2> x(1:1.7:9)
  error: rounding non-integer range used as index to nearest integer
  octave:2> isindex (1:1.7:9)
  ans = 0

in normal Octave mode, and

  >> x = 1:10
  x =

      1    2    3    4    5    6    7    8    9   10

  >> x(1:1.7:9)
  warning: rounding non-integer range used as index to nearest integer
  ans =

          1        3        4        6        8

  >> isindex (1:1.7:9)
  ans = 0

with --traditional.  The only thing I'm confused about is why isindex
should be returning false for this case when using the value as an
index works.  But isn't that what your patch did as well?

jwe


# HG changeset patch
# User Jaroslav Hajek <address@hidden>
# Date 1272577979 14400
# Node ID c919d07ac8236b390e49b2ece224a96cd42312dd
# Parent  eca2af8a92bcc5dc7a02b42455766b514b38556a
allow non-integer ranges as indices conditionally

diff --git a/src/ChangeLog b/src/ChangeLog
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,18 @@
+2010-04-29  Jaroslav Hajek  <address@hidden>
+           John W. Eaton  <address@hidden>
+
+       * utils.cc (reset_warning_state): New function.
+       (Fisindex): Temporarily set warning state for
+       Octave:allow-noninteger-ranges-as-indices to "error".
+       * error.cc (set_warning_state): New function.
+       (initialize_default_warning_state): Set default warning state
+       for Octave:allow-noninteger-ranges-as-indices to "error".
+       * error.h (set_warning_state): Provide decl.
+       * octave.cc (maximum_braindamage): Set warning state for
+       Octave:allow-noninteger-ranges-as-indices to "on".
+       * ov-range.cc (octave_range::index_vector): Warn if range
+       contains non-integer values.
+
 2010-04-27  John W. Eaton  <address@hidden>
 
        * graphics.h.in (string_array_property::string_array_property):
diff --git a/src/error.cc b/src/error.cc
--- a/src/error.cc
+++ b/src/error.cc
@@ -1441,17 +1441,23 @@
 }
 
 void
-disable_warning (const std::string& id)
+set_warning_state (const std::string& id, const std::string& state)
 {
   octave_value_list args;
 
   args(1) = id;
-  args(0) = "off";
+  args(0) = state;
 
   Fwarning (args, 0);
 }
 
 void
+disable_warning (const std::string& id)
+{
+  set_warning_state (id, "off");
+}
+
+void
 initialize_default_warning_state (void)
 {
   initialize_warning_options ("on");
@@ -1473,6 +1479,10 @@
   disable_warning ("Octave:string-concat");
   disable_warning ("Octave:variable-switch-label");
   disable_warning ("Octave:complex-cmp-ops");
+
+  // This should be an error unless we are in maximum braindamage mode.
+
+  set_warning_state ("Octave:allow-noninteger-ranges-as-indices", "error");
 }
 
 DEFUN (lasterror, args, ,
diff --git a/src/error.h b/src/error.h
--- a/src/error.h
+++ b/src/error.h
@@ -94,6 +94,9 @@
 // Helper function for print_usage defined in defun.cc.
 extern OCTINTERP_API void defun_usage_message (const std::string& msg);
 
+extern OCTINTERP_API void
+set_warning_state (const std::string& id, const std::string& state);
+
 extern OCTINTERP_API void disable_warning (const std::string& id);
 extern OCTINTERP_API void initialize_default_warning_state (void);
 
diff --git a/src/octave.cc b/src/octave.cc
--- a/src/octave.cc
+++ b/src/octave.cc
@@ -573,6 +573,8 @@
   bind_internal_variable ("page_screen_output", false);
   bind_internal_variable ("print_empty_dimensions", false);
 
+  set_warning_state ("Octave:allow-noninteger-ranges-as-indices", "on");
+
   disable_warning ("Octave:abbreviated-property-match");
   disable_warning ("Octave:fopen-file-in-path");
   disable_warning ("Octave:function-name-clash");
diff --git a/src/ov-range.cc b/src/ov-range.cc
--- a/src/ov-range.cc
+++ b/src/ov-range.cc
@@ -30,6 +30,8 @@
 #include "lo-ieee.h"
 #include "lo-utils.h"
 
+#include "defun.h"
+#include "variables.h"
 #include "gripes.h"
 #include "ops.h"
 #include "oct-obj.h"
@@ -142,6 +144,25 @@
     }
 }
 
+idx_vector
+octave_range::index_vector (void) const
+{
+  if (idx_cache)
+    return *idx_cache;
+  else
+    {
+      if (range.all_elements_are_ints ())
+        return set_idx_cache (idx_vector (range));
+      else
+        {
+          warning_with_id ("Octave:allow-noninteger-ranges-as-indices",
+                           "rounding non-integer range used as index to 
nearest integer");
+
+          return octave_value (matrix_value ()).round ().index_vector ();
+        }
+    }
+}
+
 double
 octave_range::double_value (bool) const
 {
diff --git a/src/ov-range.h b/src/ov-range.h
--- a/src/ov-range.h
+++ b/src/ov-range.h
@@ -106,8 +106,7 @@
   octave_value do_index_op (const octave_value_list& idx,
                             bool resize_ok = false);
 
-  idx_vector index_vector (void) const 
-    { return idx_cache ? *idx_cache : set_idx_cache (idx_vector (range)); }
+  idx_vector index_vector (void) const;
 
   dim_vector dims (void) const
     { 
diff --git a/src/utils.cc b/src/utils.cc
--- a/src/utils.cc
+++ b/src/utils.cc
@@ -60,6 +60,7 @@
 #include "oct-hist.h"
 #include "oct-obj.h"
 #include "pager.h"
+#include "parse.h"
 #include "sysdep.h"
 #include "toplev.h"
 #include "unwind-prot.h"
@@ -1303,6 +1304,17 @@
     }
 }
 
+// FIXME -- is there some way to fix the declarations in unwind-prot.h
+// so that this function's argument can be declared as
+// "const octave_value_list&"?
+
+static void
+reset_warning_state (octave_value_list args)
+{
+  if (! args.empty ())
+    feval ("warning", args, 0);
+}
+
 DEFUN (isindex, args, ,
   "-*- texinfo -*-\n\
 @deftypefn {Built-in Function} {} isindex (@var{ind}, @var{n})\n\
@@ -1325,7 +1337,18 @@
   if (! error_state)
     {
       unwind_protect frame;
+
+      octave_value_list xargs;
+
+      xargs(1) = "Octave:allow-noninteger-ranges-as-indices";
+      xargs(0) = "error";
+
+      octave_value_list current_warning_state = feval ("warning", xargs, 1);
+
+      frame.add_fcn (reset_warning_state, current_warning_state);
+
       frame.protect_var (error_state);
+
       frame.protect_var (discard_error_messages);
       discard_error_messages = true;