<ranges>: Fix zip_transform_view by not using possibly ill-formed static data member
#4416
Conversation
frederick-vs-ja
force-pushed
the
fix-zip_transform_view-const-iteration
branch
from
b10549d to
e814b2e
Compare
frederick-vs-ja
force-pushed
the
fix-zip_transform_view-const-iteration
branch
from
e814b2e to
52f8c07
Compare
| @@ -858,6 +872,10 @@ int main() { | |||
| test_one(three_element_transform_closure, three_range_transform_results_array, test_element_array_one, | |||
| test_element_array_two, test_element_array_three); | |||
| } | |||
| { | |||
| STATIC_ASSERT(test_gh_4414()); | |||
There was a problem hiding this comment.
(Pre-existing) I'm wondering why would C++20 tests use STATIC_ASSERT macro rather than terse static_assert
There was a problem hiding this comment.
It stringifies the condition. MSVC doesn't report the condition (/diagnostics:caret reports it indirectly, but we currently don't use that in our tests - I think we did in the past):
C:\Temp>type woof.cpp
static_assert(17 == 29);
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /c woof.cpp
woof.cpp
woof.cpp(1): error C2607: static assertion failed
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /c /diagnostics:caret woof.cpp
woof.cpp
woof.cpp(1,18): error C2607: static assertion failed
static_assert(17 == 29);
^
C:\Temp>clang-cl /EHsc /nologo /W4 /std:c++latest /c woof.cpp
woof.cpp(1,15): error: static assertion failed due to requirement '17 == 29'
1 | static_assert(17 == 29);
| ^~~~~~~~
1 error generated.
IIRC, other maintainers liked the stringifying, whereas I didn't care - unlike runtime assertions (which can be sporadic, so it's helpful to capture in logs what failed, even if it's technically possible to go look up line numbers), static_asserts tend not to start failing, or if they do it's because of constexpr evaluation in which case the line isn't actually interesting, so I have to build the test case myself anyways, in which case I can see the line right there.
I think the status quo is that some, but not all, C++20-and-above tests use the STATIC_ASSERT macro while others use terse static_assert, reflecting the strength of maintainer feelings about this.
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
🤐 🪄 😻 |
Fixes #4414.
While there's DevCom-1603671 which directly results in the reported error, the library implementation is also currently somehow broken - the initializer of
_Enable_const_begin_endmay be ill-formed when the value is expected to befalse, which can makeranges::range<const ranges::zip_transform_view<...>>ill-formed when it should befalse.