This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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;
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]