<mdspan>: Fixes signed overflow in _Fwd_prod_of_extents#4037
<mdspan>: Fixes signed overflow in _Fwd_prod_of_extents#4037StephanTLavavej merged 3 commits intomicrosoft:mainfrom
<mdspan>: Fixes signed overflow in _Fwd_prod_of_extents#4037Conversation
| typename _Extents::size_type _Result = 1; | ||
| 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.
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.
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.