-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
<mdspan>
: Fixes signed overflow in _Fwd_prod_of_extents
#4037
<mdspan>
: Fixes signed overflow in _Fwd_prod_of_extents
#4037
Conversation
for (size_t _Dim = 0; _Dim < _Idx; ++_Dim) { | ||
_Result *= _Exts.extent(_Dim); | ||
_Result *= static_cast<_Extents::size_type>(_Exts.extent(_Dim)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the static_cast
here is redundant because it will be covered by [expr.arith.conv]/p1.4.1 or p1.4.3.1, either of which results in size_type
being the common type. But I opted for clarity of the desired behavior.
} | ||
return _Result; | ||
|
||
return static_cast<_Extents::index_type>(_Result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may result in a negative number if index_type
is signed, but I think the only way that can happen without a precondition violation is in the call from mdspan::size
. This is a well-defined conversion from C++20 on, and casting back to size_type
, which size()
does, recovers the original product.
This is the reason that the specialization below, for the rank_dynamic() == 0
case, works as-is.
tests/std/tests/GH_004023_mdspan_fwd_prod_overflow/test.compile.pass.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004023_mdspan_fwd_prod_overflow/test.compile.pass.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004023_mdspan_fwd_prod_overflow/test.compile.pass.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004023_mdspan_fwd_prod_overflow/test.compile.pass.cpp
Show resolved
Hide resolved
tests/std/tests/GH_004023_mdspan_fwd_prod_overflow/test.compile.pass.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004023_mdspan_fwd_prod_overflow/test.compile.pass.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004023_mdspan_fwd_prod_overflow/test.compile.pass.cpp
Outdated
Show resolved
Hide resolved
Thanks! FYI @CaseyCarter I pushed trivial test changes after you approved. |
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Thanks for fixing this bug before it shipped to users! 🐞 🎉 🚀 |
Fixes #4023.