You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by "Travis Vitek (JIRA)" <ji...@apache.org> on 2008/01/10 23:07:34 UTC

[jira] Created: (STDCXX-693) std::valarray::sum does not work correctly for udt that has nonzero default value

std::valarray::sum does not work correctly for udt that has nonzero default value
---------------------------------------------------------------------------------

                 Key: STDCXX-693
                 URL: https://issues.apache.org/jira/browse/STDCXX-693
             Project: C++ Standard Library
          Issue Type: Bug
          Components: 26. Numerics
    Affects Versions: 4.2.0
            Reporter: Travis Vitek
            Priority: Minor


#include <cassert>
#include <valarray>

struct S
{
    // this ctor should not be required
    S ()
        : value (21)
    {
    }

    S (int v)
        : value (v)
    {
    }

    S (const S& rhs)
        : value (rhs.value)
    {
    }

    S& operator+= (const S& rhs)
    {
        value += rhs.value;
        return *this;
    }

    int value;
};

int main ()
{
    const std::valarray<S> b (S (10), 1); // 10 elements with value 1
    assert (b.sum ().value == 10);

    return 0;
} 

The wording in the standard seems to imply that the returned value should be a copy of one of the elements, and that op+= should be called on all of the other elements. If this is the case, then this an additional issue that would be detectable in user code [the user can count how many times the op+= is invoked]. This issue may apply to the min() and max() methods as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (STDCXX-693) std::valarray::sum does not work correctly for udt that has nonzero default value

Posted by "Travis Vitek (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/STDCXX-693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12557927#action_12557927 ] 

Travis Vitek commented on STDCXX-693:
-------------------------------------

Yes, I like that solution also. I've already done this for most of <valarray> for STDCXX-622 [which, I just noticed, appears to be a duplicate of STDCXX-512].

> std::valarray::sum does not work correctly for udt that has nonzero default value
> ---------------------------------------------------------------------------------
>
>                 Key: STDCXX-693
>                 URL: https://issues.apache.org/jira/browse/STDCXX-693
>             Project: C++ Standard Library
>          Issue Type: Bug
>          Components: 26. Numerics
>    Affects Versions: 4.2.0
>            Reporter: Travis Vitek
>            Priority: Minor
>
> #include <cassert>
> #include <valarray>
> struct S
> {
>     // this ctor should not be required
>     S ()
>         : value (21)
>     {
>     }
>     S (int v)
>         : value (v)
>     {
>     }
>     S (const S& rhs)
>         : value (rhs.value)
>     {
>     }
>     S& operator+= (const S& rhs)
>     {
>         value += rhs.value;
>         return *this;
>     }
>     int value;
> };
> int main ()
> {
>     const std::valarray<S> b (S (10), 1); // 10 elements with value 1
>     assert (b.sum ().value == 10);
>     return 0;
> } 
> The wording in the standard seems to imply that the returned value should be a copy of one of the elements, and that op+= should be called on all of the other elements. If this is the case, then this an additional issue that would be detectable in user code [the user can count how many times the op+= is invoked]. This issue may apply to the min() and max() methods as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (STDCXX-693) std::valarray::sum does not work correctly for udt that has nonzero default value

Posted by "Martin Sebor (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/STDCXX-693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12557908#action_12557908 ] 

Martin Sebor commented on STDCXX-693:
-------------------------------------

I agree that the wording in the standard does require it, although I suspect this interpretation was not intended (I could be wrong). GNU libstdc++ behaves the same as stdcxx, but STLport does what you expect. The straightforward fix is to copy the first element to the accumulator:

Index: include/valarray
===================================================================
--- include/valarray    (revision 609201)
+++ include/valarray    (working copy)
@@ -174,7 +174,7 @@
 
     // 26.3.2.7, p2
     value_type sum () const {
-        return accumulate (_C_array.begin (), _C_array.end (), value_type ());
+        return accumulate (_C_array.begin () + 1, _C_array.end (), _C_array [0]);
     }
 
     // 26.3.2.7, p3


Rather than implementing it, I would like to replace the use of std::accumulate() with a simple loop like so:


Index: include/valarray
===================================================================
--- include/valarray    (revision 609201)
+++ include/valarray    (working copy)
@@ -173,9 +173,7 @@
     }
 
     // 26.3.2.7, p2
-    value_type sum () const {
-        return accumulate (_C_array.begin (), _C_array.end (), value_type ());
-    }
+    value_type sum () const;
 
     // 26.3.2.7, p3
     value_type (min)() const {

Index: include/valarray.cc
===================================================================
--- include/valarray.cc (revision 609201)
+++ include/valarray.cc (working copy)
@@ -31,6 +31,18 @@
 
 
 template <class _TypeT>
+_TypeT valarray<_TypeT>::sum () const
+{
+    _TypeT __sum (_C_array [0]);
+
+    for (_RWSTD_SIZE_T __i = 1; __i < _C_array.size (); ++__i)
+        __sum += _C_array [__i];
+
+    return __sum;
+}
+
+
+template <class _TypeT>


> std::valarray::sum does not work correctly for udt that has nonzero default value
> ---------------------------------------------------------------------------------
>
>                 Key: STDCXX-693
>                 URL: https://issues.apache.org/jira/browse/STDCXX-693
>             Project: C++ Standard Library
>          Issue Type: Bug
>          Components: 26. Numerics
>    Affects Versions: 4.2.0
>            Reporter: Travis Vitek
>            Priority: Minor
>
> #include <cassert>
> #include <valarray>
> struct S
> {
>     // this ctor should not be required
>     S ()
>         : value (21)
>     {
>     }
>     S (int v)
>         : value (v)
>     {
>     }
>     S (const S& rhs)
>         : value (rhs.value)
>     {
>     }
>     S& operator+= (const S& rhs)
>     {
>         value += rhs.value;
>         return *this;
>     }
>     int value;
> };
> int main ()
> {
>     const std::valarray<S> b (S (10), 1); // 10 elements with value 1
>     assert (b.sum ().value == 10);
>     return 0;
> } 
> The wording in the standard seems to imply that the returned value should be a copy of one of the elements, and that op+= should be called on all of the other elements. If this is the case, then this an additional issue that would be detectable in user code [the user can count how many times the op+= is invoked]. This issue may apply to the min() and max() methods as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (STDCXX-693) std::valarray::sum does not work correctly for udt that has nonzero default value

Posted by "Martin Sebor (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/STDCXX-693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12558018#action_12558018 ] 

Martin Sebor commented on STDCXX-693:
-------------------------------------

FWIW, I also did most of this as part of the "big valarray rewrite" last year. None of it is checked in because it's binary incompatible. That said, I have no problem committing improvements to valarray piecemeal, just as long as they're compatible, and leaving the breaking changes for last (5.0).

Since it sounds like we've both made some of the same changes it might make sense to compare our approaches. If you agree, I suggest you post your patch to the list before committing it to give me a chance to review it.

Incidentally, since we're outlining a function in the patch,  it wouldn't be a forward compatible change, would it?

> std::valarray::sum does not work correctly for udt that has nonzero default value
> ---------------------------------------------------------------------------------
>
>                 Key: STDCXX-693
>                 URL: https://issues.apache.org/jira/browse/STDCXX-693
>             Project: C++ Standard Library
>          Issue Type: Bug
>          Components: 26. Numerics
>    Affects Versions: 4.2.0
>            Reporter: Travis Vitek
>            Priority: Minor
>
> #include <cassert>
> #include <valarray>
> struct S
> {
>     // this ctor should not be required
>     S ()
>         : value (21)
>     {
>     }
>     S (int v)
>         : value (v)
>     {
>     }
>     S (const S& rhs)
>         : value (rhs.value)
>     {
>     }
>     S& operator+= (const S& rhs)
>     {
>         value += rhs.value;
>         return *this;
>     }
>     int value;
> };
> int main ()
> {
>     const std::valarray<S> b (S (10), 1); // 10 elements with value 1
>     assert (b.sum ().value == 10);
>     return 0;
> } 
> The wording in the standard seems to imply that the returned value should be a copy of one of the elements, and that op+= should be called on all of the other elements. If this is the case, then this an additional issue that would be detectable in user code [the user can count how many times the op+= is invoked]. This issue may apply to the min() and max() methods as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.