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

Compiler bug workaround for using format() in a user-defined module #4919

Merged
merged 3 commits into from
Aug 28, 2024

Conversation

StephanTLavavej
Copy link
Member

DevCom-10313766 VSO-1775715 "Using std::format in a module requires including <format> header in .cpp files using that module" is caused by a compiler bug where the compiler is currently unable to encode UDLs in the IFC. The workaround is to avoid UDLs. Constructing temporaries of length 2 or 1 was the simplest, compared to extracting named constexpr variables.

Since the workaround is relatively simple but affects several lines, I chose to make it unconditional (instead of guarding it for MSVC only), and I commented only the first occurrence (no reason to repeat it on every line). My hope is that the compiler will be fixed very soon (@cdacamar is investigating).

I audited the STL for other UDLs in our headers, and <filesystem> was the only other user of UDLs. However, all of its usage was in non-templated functions, which appear to be immune to the compiler bug, so I don't believe that <filesystem> changes are necessary.

This scenario, where the STL is included in the Global Module Fragment of a user-defined module, is clearly under-explored - along the way I encountered and reported the much more severe VSO-2227713 "Modules: Exported function templates can't use std::cout properly" (with no known workaround). So, I'm adding a dedicated test for this scenario, currently just exercising the format() scenario but available to extend in the future. I had to extract P2465R3_standard_library_modules/env.lst (because we have to handle the usually-force-included machinery specially, and can't tolerate /permissive, we need dedicated configurations).

This requires special Python and Perl machinery to compile the user.ixx before the test.cpp, but very similar to existing tests that we have elsewhere. I verified both the GitHub Python and internal Perl test harnesses, both positively (test passes with product changes) and negatively (test fails without product changes).

Verified that without the product code workaround, this fails with:

D:\GitHub\STL\out\x64\out\inc\format(2635): error C3688: invalid literal suffix 'sv'; literal operator or literal operator template 'operator ""sv' not found
D:\GitHub\STL\out\x64\out\inc\format(2637): error C3688: invalid literal suffix 'sv'; literal operator or literal operator template 'operator ""sv' not found
D:\GitHub\STL\out\x64\out\inc\format(2639): error C3688: invalid literal suffix 'sv'; literal operator or literal operator template 'operator ""sv' not found
D:\GitHub\STL\out\x64\out\inc\format(2641): error C3688: invalid literal suffix 'sv'; literal operator or literal operator template 'operator ""sv' not found
D:\GitHub\STL\out\x64\out\inc\format(2644): error C3688: invalid literal suffix 'sv'; literal operator or literal operator template 'operator ""sv' not found

Perl also tested positively and negatively.
@StephanTLavavej StephanTLavavej added bug Something isn't working format C++20/23 format modules C++23 modules, C++20 header units labels Aug 27, 2024
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner August 27, 2024 01:20
@CaseyCarter CaseyCarter removed their assignment Aug 27, 2024
@StephanTLavavej StephanTLavavej self-assigned this Aug 27, 2024
@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 0b6e350 into microsoft:main Aug 28, 2024
39 checks passed
@StephanTLavavej StephanTLavavej deleted the meow-sv branch August 28, 2024 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working format C++20/23 format modules C++23 modules, C++20 header units
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants