Skip to content

Conversation

@vmichal
Copy link

@vmichal vmichal commented Nov 12, 2025

Remove free function overloads of begin and end for initializer_list.
Replace free function overloads empty and data for initializer_list with member functions.
Replace free function overloads begin and end for valarray with member functions.
Provide new feature test macros __cpp_lib_initializer_list and __cpp_lib_valarray.

Resolves #5840

@vmichal vmichal requested a review from a team as a code owner November 12, 2025 14:23
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Nov 12, 2025
@vmichal
Copy link
Author

vmichal commented Nov 12, 2025

@microsoft-github-policy-service agree

@vmichal
Copy link
Author

vmichal commented Nov 12, 2025

Sections 4.1, most of 4.7 and 4.8 of the paper prescribe changes this STL already has (some #includes and noexcept/conditionally noexcept functions), so there was nothing to do.

Section 4.9 rewords the effects of some standard library member functions (mostly members of std::string), but they seem not relevant for the library implementation. Wording of https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3016r6.html#wording-support.initlist defines data() as identical to begin(), so redefinition of other operations previously defined using begin() to data() has no effect IMO.
Do we want to actually change calls to _Ilist.begin() to _Ilist.data()? There are many other places, e.g. in <xstring>, where the (begin, size) pair is used instead of (data, size) required by the paper..

@vmichal
Copy link
Author

vmichal commented Nov 12, 2025

There are two failing tests (copied from CI):

std :: tests/Dev11_1074023_constexpr:37 (3864 of 7663)
libc++ :: std/language.support/support.initlist/support.initlist.range/begin_end.pass.cpp

Dev11_1074023_constexpr fails on line 56

STATIC_ASSERT(begin(il) == end(il));

The test is compiled in C++14 mode. The removed non-member overload begin(initializer_list) used to be constexpr since C++14 (according to https://en.cppreference.com/w/cpp/utility/initializer_list/begin2.html), whereas the primary template wouldn't be constexpr until C++17 (https://en.cppreference.com/w/cpp/iterator/begin.html). Same issue for the libc++ test.

It appears that we either can't implement this change unconditionally, or we have to drop _CONSTEXPR17 from primary templates in <xutility> and replace it with unconditional constexpr. The third alternative (removing the failing test) does not seem appropriate.

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Nov 12, 2025

For Dev11_1074023_constexpr, we can simply make that line of the test conditional on _HAS_CXX17.

For libc++, we can use tests/libcxx/expected_results.txt to mark the test as a known FAIL case until libc++ updates their implementation and corresponding test.

Do we want to actually change calls to _Ilist.begin() to _Ilist.data()?

Under the As If Rule, we aren't required to, and I don't see the need to change this.

@StephanTLavavej StephanTLavavej added the cxx26 C++26 feature label Nov 12, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Work In Progress in STL Code Reviews Nov 12, 2025
Only check that begin(initializer_list) is constexpr in C++17 onwards
(tests/Dev11_1074023_constexpr).

Mark libc++ std/language.support/support.initlist/support.initlist.range/begin_end.pass.cpp
as known FAIL.
@StephanTLavavej StephanTLavavej self-assigned this Nov 12, 2025
@StephanTLavavej StephanTLavavej moved this from Work In Progress to Initial Review in STL Code Reviews Nov 12, 2025
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

RUNALL_INCLUDE ..\usual_latest_matrix.lst
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes should be tested in C++14 mode.

Suggested change
RUNALL_INCLUDE ..\usual_latest_matrix.lst
RUNALL_INCLUDE ..\usual_matrix.lst

Also, static_assert(e); is only standardized since C++17, so we need to write either static_assert(e, ""); or #define STATIC_ASSERT(__VA_ARGS__) static_assert(__VA_ARGS__, #__VA_ARGS__). The latter is widely used in MSVC STL's test suite.

Comment on lines 1837 to 1839
// C++26
#define __cpp_lib_initializer_list 202511L
#define __cpp_lib_valarray 202511L
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be moved to the C++14 section. And we should test them in tests/std/tests/VSO_0157762_feature_test_macros/test.compile.pass.cpp.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. Individual parts of yvals_core.h are sorted alphabetically, right? So I should prefer alphabetical sorting over smaller diff?
Tests in tests/std/tests/VSO_0157762_feature_test_macros/test.compile.pass.cpp don't seem to be sorted, so I will just add new lines to the end.

STATIC_ASSERT(il.begin() == il.end());
#if _HAS_CXX17
STATIC_ASSERT(begin(il) == end(il));
#endif // _HAS_CXX17
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For archaeology: constexpr for initializer_list was added to C++14 via WG21-N3471, while constexpr for free std::begin/std::end was added to C++17 via WG21-P0031R0.

I'm a bit hesitant now as the changes would disallow executing for (int n : {1, 2, 3}) { /* ... */ } in constant evaluation in C++14 mode, while WG21-N3471 intentionally allowed this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No language lawyer here, so correct me if I am wrong, but the range-based for loop does not use the free function in this case, but rather the member begin/end. Those are marked as constexpr unconditionally (not only in C++14 onwards) in code.

Link to cppinsights C++14 mode (very powerful argument for normal developers, not sure whether it is appropriate for STL development 😅 ) https://cppinsights.io/s/faddcfbd

Nevertheless, I have added the for loop to the test suite and it compiles and executes in all modes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No language lawyer here, so correct me if I am wrong, but the range-based for loop does not use the free function in this case, but rather the member begin/end. Those are marked as constexpr unconditionally (not only in C++14 onwards) in code.

Oh, you are right. Sorry. I must have been be confused with some constexpr reduction.

@StephanTLavavej
Copy link
Member

Status update (also for #5848): Your PRs are on my list of things to review (the STL Code Reviews project is my dashboard) but I will be delayed as I need to prioritize <flat_map> and <flat_set> soon. Just wanted to let you know that I'm not forgetting about them.

@StephanTLavavej StephanTLavavej removed their assignment Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cxx26 C++26 feature

Projects

Status: Initial Review

Development

Successfully merging this pull request may close these issues.

P3016R6 Resolve Inconsistencies In begin/end For valarray And Braced Initializer Lists

3 participants