Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix modules test by using static constexpr arrays #3588

Merged
merged 1 commit into from Mar 24, 2023

Conversation

StephanTLavavej
Copy link
Member

This mirrors the library test changes from @JonCavesMSFT's MSVC-PR-460102.

In the header units and named modules test, I was unintentionally being a bad kitty 馃樇 and was exercising an accepts-invalid codepath in the MSVC compiler, which is now being fixed. Specifically, WG21-N4928 [expr.const]/12.2:

A constant expression is either a glvalue core constant expression that refers to an entity that is a permitted result of a constant expression (as defined below), or a prvalue core constant expression whose value satisfies the following constraints:
[...]
if the value is of pointer type, it contains the address of an object with static storage duration, the address past the end of such an object (7.6.6), the address of a non-immediate function, or a null pointer value,

In my test code, I was constructing constexpr reverse_iterators and spans by taking the address of a non-static constexpr array at function scope. This is forbidden; the array must be static constexpr for it to work.

With this change, the code is now conformant, and accepted by both the old and new compilers, so we can make the change simultaneously in MSVC and GitHub. Thanks Jonathan! 馃樆

(It's worth noting that both Clang and GCC correctly enforce this rule. However, the header units and named modules test currently runs for MSVC only, not EDG or Clang, which is why I missed this.)

@StephanTLavavej StephanTLavavej added test Related to test code modules C++23 modules, C++20 header units labels Mar 23, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner March 23, 2023 21:45
@github-actions github-actions bot added this to Initial Review in Code Reviews Mar 23, 2023
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in Code Reviews Mar 23, 2023
@strega-nil-ms strega-nil-ms moved this from Final Review to Ready To Merge in Code Reviews Mar 24, 2023
@StephanTLavavej StephanTLavavej merged commit 5e60382 into microsoft:main Mar 24, 2023
37 checks passed
Code Reviews automation moved this from Ready To Merge to Done Mar 24, 2023
@StephanTLavavej StephanTLavavej deleted the static-constexpr branch March 24, 2023 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules C++23 modules, C++20 header units test Related to test code
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants