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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a Standard mode check to <__msvc_print.hpp> #4111

Merged
merged 5 commits into from
Oct 25, 2023

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Oct 20, 2023

<__msvc_print.hpp> was missing the Standard mode check that the rest of our internal headers perform. This one's a little special because it's included by <format> in C++20 Concepts mode, and by <print> in C++23 Concepts mode.

Without this check, the header is still rejected in C++14/17 mode, but with a less clear error about the usage of consteval:

_NODISCARD consteval bool _Is_ordinary_literal_encoding_utf8() {

Also, this moves the other Standard mode checks in <xmeow.h> as early as possible. The mode is available after including <yvals_core.h>, and if we're doomed, we should diagnose doom immediately. <xcharconv_tables.h> was already doing this.

Next, <ostream> was guarding its inclusion of <__msvc_print.hpp> with #if _HAS_CXX23, even though its usage was guarded with #ifdef __cpp_lib_print:

#ifdef __cpp_lib_print

This meant that in our test suite, EDG in C++23 non-concepts mode had <ostream> including <__msvc_print.hpp> and then doing nothing with it. That was previously harmless but is now an error, so we need to make the guards consistent. (I don't think <ostream>'s dependency needs to be mentioned in the <__msvc_print.hpp> error message, that would just be confusing and it wouldn't be why users were trying to include the thing.)

Finally, GH_001411_core_headers was guarding with #if _HAS_CXX23, which was both too restrictive (the header is used by <format> in C++20) and too permissive (the header is used only in Concepts mode). We need to change this to #ifdef __cpp_lib_concepts.

Thanks to @ThomasFWise for the report!

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Oct 20, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner October 20, 2023 21:56
@StephanTLavavej StephanTLavavej self-assigned this Oct 20, 2023
@StephanTLavavej StephanTLavavej removed their assignment Oct 20, 2023
stl/inc/__msvc_print.hpp Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter removed their assignment Oct 24, 2023
@StephanTLavavej StephanTLavavej self-assigned this Oct 24, 2023
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 43fd331 into microsoft:main Oct 25, 2023
35 checks passed
@StephanTLavavej StephanTLavavej deleted the msvc-print branch October 25, 2023 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants