Re: [[patch] valarray (was Re: valarray_copy implementation question)

[Christian BRUEL <christian dot bruel at st dot com>]

Hello Paolo,

First, a typo, in my example the assignment should have been
myarray[mymask] += valarray<int>(3,n);  // with n = 10 which is > as n 
true elements.
(I'm reattaching it here with the correct size and removing the 
testsuite dependencies to make it standalone).

So now, I think that your interpretation of the Standard is wrong when 
you write :  "the source array is supposed to have exactly as many 
elements as the true booleans on the left side"

that is an undefined behavior, (see INCITS ISO/IEC 14882-2003 Section 
26.3.2.6 on valarray computed assigments):
Quote: "If the array and the argument array do not have the same length, 
the behavior is undefined",
if you consider the possibility than there are less n true elements than 
the size of the source array, which is the case here.

So now back to my example, when I do

  mymask[0] = true;
  mymask[9] = true;

  myarray[mymask] += valarray<int>(3,n)

I really expects (myarray[0] == 4 and (myarray[9]) to be incremented, 
with myarray of size n=10 initialized with 1.

and your assignment proposals:
myarray[mymask] += valarray<int>(3,0);
myarray[mymask] += valarray<int>(3,2);
are NOT guaranteed to work fine since you'd have the array to be of size 
10 and the argument array of size [0,2]. Note that the size of myarray 
is really 10, it's not the number of true elements that only determines 
the elements to which the operation will apply.

But in anycase, the current trunk behavior of the testcase is 
Segmentation fault (compile with -O0) which is obviously wrong.

Do we agree now ?

Best Regards

Christian

Paolo Carlini wrote:
> Hi,
>
> to be honest I'm not at all convinced the testcase you are proposing is
> consistent with our interpretation of the Standard: as I remember the
> issue, the source array is supposed to have exactly as many elements as
> the true booleans on the left side, thus only a testcase with
>    
>     myarray[mymask] += valarray<int>(3,0);
>
> and
>    
>     myarray[mymask] += valarray<int>(3,2);
>
> respectively, is guaranteed to work fine.  I understand however that it
> would be convenient to allow for a larger right hand side array, makes
> easier writing this kind of code, Gaby has the last word, if he believes
> that it can be implemented in a safe and consistent way wrt the rest of
> valarray, I have no objections of course to your patch. In case,
> remember to also add to the testsuite the other testcase I sent the last
> time, and format and style both properly, see the other testcases in the
> tessuite for examples, thus: separate test01 function called from main,
> a test variable on top with attribute unused, VERIFY( XX ), not VERIFY
> (XX), in general function calls like f(y), not f (y), etc.
>
> Paolo.
// segfaults on x86 and infinite loop on SH.

#include <valarray>
#include <cassert>

using namespace std;

void test01()
{
  int n = 10;

  valarray<int> myarray (1, n);

  valarray<bool> mymask (n);
  myarray[mymask] += valarray<int>(3,n);

  for (int i = 0; i < n; i++)
    assert (myarray[i] == 1);
    
  mymask[0] = true;
  mymask[9] = true;

  myarray[mymask] += valarray<int>(3,n); 

  assert(myarray[0] == 4);
  assert(myarray[9] == 4);
  assert(myarray[1] == 1);
  assert(myarray[2] == 1);
  assert(myarray[3] == 1);
  assert(myarray[4] == 1);
  assert(myarray[5] == 1);
  assert(myarray[6] == 1);
  assert(myarray[7] == 1);
  assert(myarray[8] == 1);
}

int main()
{
  test01();
  return 0;
}