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.

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