[PATCH] D41992: [libcxx] Avoid spurious construction of valarray elements
This revision was automatically updated to reflect the committed changes. Closed by commit rCXX324596: [libcxx] Avoid spurious construction of valarray elements (authored by miyuki, committed by ). Repository: rCXX libc++ https://reviews.llvm.org/D41992 Files: include/valarray test/std/numerics/numarray/template.valarray/valarray.assign/copy_assign.pass.cpp test/std/numerics/numarray/template.valarray/valarray.assign/initializer_list_assign.pass.cpp test/std/numerics/numarray/template.valarray/valarray.cons/default.pass.cpp test/std/numerics/numarray/template.valarray/valarray.cons/size.pass.cpp Index: include/valarray === --- include/valarray +++ include/valarray @@ -1053,6 +1053,9 @@ friend const _Up* end(const valarray<_Up>& __v); + +void __clear(); +valarray& __assign_range(const value_type* __f, const value_type* __l); }; _LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::valarray(size_t)) @@ -2750,7 +2753,24 @@ : __begin_(0), __end_(0) { -resize(__n); +if (__n) +{ +__begin_ = __end_ = static_cast(_VSTD::__allocate(__n * sizeof(value_type))); +#ifndef _LIBCPP_NO_EXCEPTIONS +try +{ +#endif // _LIBCPP_NO_EXCEPTIONS +for (; __n; --__n, ++__end_) +::new (__end_) value_type(); +#ifndef _LIBCPP_NO_EXCEPTIONS +} +catch (...) +{ +__clear(); +throw; +} +#endif // _LIBCPP_NO_EXCEPTIONS +} } template @@ -2780,7 +2800,7 @@ } catch (...) { -resize(0); +__clear(); throw; } #endif // _LIBCPP_NO_EXCEPTIONS @@ -2805,7 +2825,7 @@ } catch (...) { -resize(0); +__clear(); throw; } #endif // _LIBCPP_NO_EXCEPTIONS @@ -2842,7 +2862,7 @@ } catch (...) { -resize(0); +__clear(); throw; } #endif // _LIBCPP_NO_EXCEPTIONS @@ -2870,7 +2890,7 @@ } catch (...) { -resize(0); +__clear(); throw; } #endif // _LIBCPP_NO_EXCEPTIONS @@ -2899,7 +2919,7 @@ } catch (...) { -resize(0); +__clear(); throw; } #endif // _LIBCPP_NO_EXCEPTIONS @@ -2928,7 +2948,7 @@ } catch (...) { -resize(0); +__clear(); throw; } #endif // _LIBCPP_NO_EXCEPTIONS @@ -2957,7 +2977,7 @@ } catch (...) { -resize(0); +__clear(); throw; } #endif // _LIBCPP_NO_EXCEPTIONS @@ -2968,30 +2988,43 @@ inline valarray<_Tp>::~valarray() { -resize(0); +__clear(); } template valarray<_Tp>& -valarray<_Tp>::operator=(const valarray& __v) +valarray<_Tp>::__assign_range(const value_type* __f, const value_type* __l) { -if (this != &__v) +size_t __n = __l - __f; +if (size() != __n) { -if (size() != __v.size()) -resize(__v.size()); -_VSTD::copy(__v.__begin_, __v.__end_, __begin_); +__clear(); +__begin_ = static_cast(_VSTD::__allocate(__n * sizeof(value_type))); +__end_ = __begin_ + __n; +_VSTD::uninitialized_copy(__f, __l, __begin_); +} else { +_VSTD::copy(__f, __l, __begin_); } return *this; } +template +valarray<_Tp>& +valarray<_Tp>::operator=(const valarray& __v) +{ +if (this != &__v) +return __assign_range(__v.__begin_, __v.__end_); +return *this; +} + #ifndef _LIBCPP_CXX03_LANG template inline valarray<_Tp>& valarray<_Tp>::operator=(valarray&& __v) _NOEXCEPT { -resize(0); +__clear(); __begin_ = __v.__begin_; __end_ = __v.__end_; __v.__begin_ = nullptr; @@ -3004,10 +3037,7 @@ valarray<_Tp>& valarray<_Tp>::operator=(initializer_list __il) { -if (size() != __il.size()) -resize(__il.size()); -_VSTD::copy(__il.begin(), __il.end(), __begin_); -return *this; +return __assign_range(__il.begin(), __il.end()); } #endif // _LIBCPP_CXX03_LANG @@ -3680,15 +3710,22 @@ template void -valarray<_Tp>::resize(size_t __n, value_type __x) +valarray<_Tp>::__clear() { if (__begin_ != nullptr) { while (__end_ != __begin_) (--__end_)->~value_type(); _VSTD::__libcpp_deallocate(__begin_); __begin_ = __end_ = nullptr; } +} + +template +void +valarray<_Tp>::resize(size_t __n, value_type __x) +{ +__clear(); if (__n) { __begin_ = __end_ = static_cast(_VSTD::__allocate(__n * sizeof(value_type))); @@ -3702,7 +3739,7 @@ } catch (...) { -resize(0); +__clear(); throw; } #endif // _LIBCPP_NO_EXCEPTIONS Inde
[PATCH] D41992: [libcxx] Avoid spurious construction of valarray elements
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This LGTM. Comment at: include/valarray:3728 +{ +__clear(); if (__n) I thought that you had lost an exception guarantee here, but it turns out that there wasn't one before. If the allocation fails, you are left with an empty array instead of the old contents. https://reviews.llvm.org/D41992 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41992: [libcxx] Avoid spurious construction of valarray elements
miyuki added a comment. ping^3 https://reviews.llvm.org/D41992 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41992: [libcxx] Avoid spurious construction of valarray elements
miyuki added a comment. ping^2 https://reviews.llvm.org/D41992 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41992: [libcxx] Avoid spurious construction of valarray elements
miyuki added a comment. ping https://reviews.llvm.org/D41992 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41992: [libcxx] Avoid spurious construction of valarray elements
miyuki created this revision. miyuki added reviewers: EricWF, mclow.lists. Currently libc++ implements some operations on valarray by using the resize method. This method has a parameter with a default value. Because of this, valarray may spuriously construct and destruct objects of valarray's element type. This patch fixes this issue and adds corresponding test cases. https://reviews.llvm.org/D41992 Files: include/valarray test/std/numerics/numarray/template.valarray/valarray.assign/copy_assign.pass.cpp test/std/numerics/numarray/template.valarray/valarray.assign/initializer_list_assign.pass.cpp test/std/numerics/numarray/template.valarray/valarray.cons/default.pass.cpp test/std/numerics/numarray/template.valarray/valarray.cons/size.pass.cpp Index: test/std/numerics/numarray/template.valarray/valarray.cons/size.pass.cpp === --- test/std/numerics/numarray/template.valarray/valarray.cons/size.pass.cpp +++ test/std/numerics/numarray/template.valarray/valarray.cons/size.pass.cpp @@ -16,6 +16,15 @@ #include #include +struct S { +S() : x(1) {} +~S() { ++cnt_dtor; } +int x; +static size_t cnt_dtor; +}; + +size_t S::cnt_dtor = 0; + int main() { { @@ -36,4 +45,11 @@ for (int i = 0; i < 100; ++i) assert(v[i].size() == 0); } +{ +std::valarray v(100); +assert(v.size() == 100); +for (int i = 0; i < 100; ++i) +assert(v[i].x == 1); +} +assert(S::cnt_dtor == 100); } Index: test/std/numerics/numarray/template.valarray/valarray.cons/default.pass.cpp === --- test/std/numerics/numarray/template.valarray/valarray.cons/default.pass.cpp +++ test/std/numerics/numarray/template.valarray/valarray.cons/default.pass.cpp @@ -16,6 +16,13 @@ #include #include +struct S { +S() { ctor_called = true; } +static bool ctor_called; +}; + +bool S::ctor_called = false; + int main() { { @@ -34,4 +41,9 @@ std::valarray > v; assert(v.size() == 0); } +{ +std::valarray v; +assert(v.size() == 0); +assert(!S::ctor_called); +} } Index: test/std/numerics/numarray/template.valarray/valarray.assign/initializer_list_assign.pass.cpp === --- test/std/numerics/numarray/template.valarray/valarray.assign/initializer_list_assign.pass.cpp +++ test/std/numerics/numarray/template.valarray/valarray.assign/initializer_list_assign.pass.cpp @@ -19,6 +19,21 @@ #include #include +struct S +{ +S() : x_(0) { default_ctor_called = true; } +S(int x) : x_(x) {} +int x_; +static bool default_ctor_called; +}; + +bool S::default_ctor_called = false; + +bool operator==(const S& lhs, const S& rhs) +{ +return lhs.x_ == rhs.x_; +} + int main() { { @@ -55,4 +70,15 @@ assert(v2[i][j] == a[i][j]); } } +{ +typedef S T; +T a[] = {T(1), T(2), T(3), T(4), T(5)}; +const unsigned N = sizeof(a)/sizeof(a[0]); +std::valarray v2; +v2 = {T(1), T(2), T(3), T(4), T(5)}; +assert(v2.size() == N); +for (std::size_t i = 0; i < v2.size(); ++i) +assert(v2[i] == a[i]); +assert(!S::default_ctor_called); +} } Index: test/std/numerics/numarray/template.valarray/valarray.assign/copy_assign.pass.cpp === --- test/std/numerics/numarray/template.valarray/valarray.assign/copy_assign.pass.cpp +++ test/std/numerics/numarray/template.valarray/valarray.assign/copy_assign.pass.cpp @@ -17,6 +17,21 @@ #include #include +struct S +{ +S() : x_(0) { default_ctor_called = true; } +S(int x) : x_(x) {} +int x_; +static bool default_ctor_called; +}; + +bool S::default_ctor_called = false; + +bool operator==(const S& lhs, const S& rhs) +{ +return lhs.x_ == rhs.x_; +} + int main() { { @@ -56,4 +71,16 @@ assert(v2[i][j] == v[i][j]); } } +{ +typedef S T; +T a[] = {T(1), T(2), T(3), T(4), T(5)}; +const unsigned N = sizeof(a)/sizeof(a[0]); +std::valarray v(a, N); +std::valarray v2; +v2 = v; +assert(v2.size() == v.size()); +for (std::size_t i = 0; i < v2.size(); ++i) +assert(v2[i] == v[i]); +assert(!S::default_ctor_called); +} } Index: include/valarray === --- include/valarray +++ include/valarray @@ -1053,6 +1053,9 @@ friend const _Up* end(const valarray<_Up>& __v); + +void __clear(); +valarray& __assign_range(const value_type* __f, const value_type* __l); }; _LIBCPP_EXTERN_TEMPLATE(_LIBCPP_FUNC_VIS valarray::valarray(size_t)) @@ -2750,7 +2753,24 @@ : __begin_(0), __end_(0) {