[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Should xelem bounds-check (and reference-check) when debugging optio
From: |
John W. Eaton |
Subject: |
Re: Should xelem bounds-check (and reference-check) when debugging options are on? |
Date: |
Tue, 24 Jun 2014 16:00:17 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0 |
On 06/23/2014 02:06 PM, David Spies wrote:
Here's my proposed changeset for this (labeled "Added safety checks to
Array::xelem"):
http://hg.octave.org/octave-dspies/graph/
Thanks,
David
On Fri, Jun 20, 2014 at 11:42 AM, David Spies <address@hidden
<mailto:address@hidden>> wrote:
Hello,
When constructing (or accessing) matrix elements, I'd like to be
able to call a function which does bounds-checking (and in the case
of assignment, reference-count-checking with an exception if there's
more than one reference) only when debugging options are turned on.
I'm proposing adding these checks to the xelem method instead of
creating a new method for it. I can't imagine why it's necessary to
have a method that does "no checks of any kind ever! Not even when
debugging!"
What do people think of the idea?
Thanks,
David
I assume you mean changeset 8bcfea54ce19, correct?
I think it is OK to allow bounds and reference checking to be
optionally enabled for the xelem functions, but they should remain
disabled by default.
+++ b/libinterp/corefcn/jit-typeinfo.cc
@@ -287,7 +287,7 @@
make_indices (indicies, idx_count, idx);
Array<double> ret = mat->array->index (idx);
- return ret.xelem (0);
+ return (static_cast<const Array<double>&> (ret)).xelem (0);
If you need to change the const-ness of something, shouldn't you use
const_cast? Or wouldn't
const Array<double>& ret = mat->array->index (idx);
work here anyway, no cast required?
+#if defined(BOUNDS_CHECKING)
+#define BOUNDS_CHECKING_DEFINED true
+#else
+#define BOUNDS_CHECKING_DEFINED false
+#endif
It might be better to arrange to always define BOUNDS_CHECKING to 1 or
0 (or true or false) to avoid the need for this extra definition.
Instead of "check_out_of_range", perhaps use "check_index_bounds"?
+ // Check for multiple references only if asserts are enabled
+ T&
+ xelem (octave_idx_type n)
+ {
+ assert(is_unique ());
I think asserts are always enabled, aren't they? We should probably
have a configure option to enable this check. I'm not sure what name
to use.
The changes to dbleQR.cc, floatQR.cc, CmplxQR.cc, and fCmplx.cc seem
unrelated, so should be deleted from the changeset.
jwe