This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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] |
This attempts to solve some of the problems when mixing std::valarray operations and 'auto', by storing nested closure objects as values instead of references. This means we don't end up with dangling references to temporary closures that have already been destroyed. This makes the closure objects introduced by the library less error-prone, but it's still possible to write incorrect code by using temporary valarrays, e.g. std::valarray<int> f(); auto x = f() * 2; std::valarray<int> y = x; Here the closure object 'x' has a dangling reference to the temporary returned by f(). It might be possible to do something about this by overloading the operators for valarray rvalues and moving the valarray into the closure, instead of holding a const-reference. I don't plan to work on that. Performance seems to be unaffected by this patch, although I didn't test that very thoroughly. Strictly speaking this is an ABI change as it changes the size and layout of the closure types like _BinClos etc. so that their _M_expr member is at least two words, not just one for a reference. Those closure types should never appear in API boundaries or as class members (if anybody was doing that by using 'auto' it wouldn't have worked correctly anyway) so I think we can just change them, without renaming the types or moving them into an inline namespace so they mangle differently. Does anybody disagree? The PR is marked as a regression because the example in the PR used to "work" with older releases. That's probably only because they didn't optimize as aggressively and so the stack space of the expired temporaries wasn't reused (ASan still shows the bug was there in older releases even if it doesn't crash). As a regression this should be backported, but the layout changes make me pause a little when considering making the change on stable release branches. PR libstdc++/83860 * include/bits/gslice_array.h (gslice_array): Define default constructor as deleted, as per C++11 standard. * include/bits/mask_array.h (mask_array): Likewise. * include/bits/slice_array.h (slice_array): Likewise. * include/bits/valarray_after.h (_GBase::_M_expr, _IBase::_M_expr): Use _ValArrayRef for type of data members. * include/bits/valarray_before.h (_ValArrayRef): New helper for type of data members in closure objects. (_FunBase::_M_expr, _UnBase::_M_expr, _BinBase::_M_expr1) (_BinBase::_M_expr2, _BinBase2::_M_expr1, _BinBase1::_M_expr2) (_SBase::_M_expr): Use _ValArrayRef for type of data members. * testsuite/26_numerics/valarray/83860.cc: New. I'll commit this to trunk only for now, unless anybody sees a problem with the approach, or thinks the layout changes require new mangled names for the closures.
Attachment:
patch.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |