Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is the glibcxx STL incorrect in its implementation of std::valarray::sum()?

Tags:

c++

gcc

valarray

I was toying with valarrays when I hit something I consider a bug in my compiler's STL implementation. Here is the smallest example I could produce:

#include <iostream>
#include <string>
#include <vector>
#include <iomanip>
#include <valarray>

using namespace std;

int main()
{
    valarray<int> Y(0xf00d, 1);
    valarray<valarray<int>> X(Y, 1);
    cout << "Y[0]           = " << std::hex << Y[0]       << '\n';
    cout << "X[0][0]        = " << std::hex << X[0][0]    << '\n';
    cout << "X[0].size()    = " << X[0].size()            << '\n';
    cout << "X.sum().size() = " << X.sum().size()         << '\n';
}

This will output:

$ g++ -std=c++17 -O2 -Wall -pedantic -pthread main.cpp && ./a.out
Y[0]           = f00d
X[0][0]        = f00d
X[0].size()    = 1
X.sum().size() = 0

You can compile and run it at coliru

Why do I consider this a bug ? Because according to the standard (26.6.2.8)

T sum() const;

This function may only be instantiated for a type T to which operator+= can be applied. This function returns the sum of all the elements of the array. If the array has length 0, the behavior is undefined. If the array has length 1, sum() returns the value of element 0. Otherwise, the returned value is calculated by applying operator+= to a copy of an element of the array and all other elements of the array in an unspecified order.

valarray does have a += operator

So I would expect X.sum() to have the same value than X[0]. But it is clearly not the case, because its size is 0 instead of 1.

I looked in the implementation of sum() and tracked it back to this piece of code:

  //
  // Compute the sum of elements in range [__f, __l)
  // This is a naive algorithm.  It suffers from cancelling.
  // In the future try to specialize
  // for _Tp = float, double, long double using a more accurate
  // algorithm.
  //
  template<typename _Tp>
    inline _Tp
    __valarray_sum(const _Tp* __f, const _Tp* __l)
    {
      _Tp __r = _Tp();
      while (__f != __l)
        __r += *__f++;
      return __r;
    }

And we understand where the problem comes from. The code accumulates the sum into __r, but instead of initializing __r with the first item in the valarray, it is default constructed. The default constructor for valarray creates an array of size 0. And so the final result is still a valarray of size 0.

Is my understanding of the standard valid (and glibcxx STL has a bug)? Or should I be corrected?

For the record, I am using g++ 7.3.0 under cygwin, but it is reproduced on coliru which probably isn't running under cygwin...

like image 214
fjardon Avatar asked Oct 17 '18 18:10

fjardon


1 Answers

This is a bug to me. sum()

Requires: size() > 0. This function may only be instantiated for a type T to which operator+= can be applied.

and valarray does have a operator += so it qualifies. It's operator +=

Requires: size() == v.size(). Each of these operators may only be instantiated for a type T if the indicated operator can be applied to two operands of type T. The value of an element in the left-hand side of a valarray compound assignment operator does not depend on the value of another element in that left hand side.

So by doing _Tp __r = _Tp(); they generate a valarray whose size() does not equal the size of the elements thus it isn't usable with it's operator +=. A more correct implementation would be

  template<typename _Tp>
    inline _Tp
    __valarray_sum(const _Tp* __f, const _Tp* __l)
    {
      _Tp __r = *__f++; // this is okay as the function is requires size > 0.  It is the users responsibility to make sure that is the case
      while (__f != __l)
        __r += *__f++;
      return __r;
    }
like image 59
NathanOliver Avatar answered Sep 22 '22 07:09

NathanOliver