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

Avoid concepts when __cpp_lib_concepts isn't defined #1749

Merged
merged 3 commits into from
Mar 23, 2021

Conversation

StephanTLavavej
Copy link
Member

Fixes DevCom-1367531 / VSO-1292416 "Intel C++ Compiler, ICL, has compilation failure because use of concepts in header file not guarded with ifdef __cpp_lib_concepts".

Earlier related discussion: #919 (comment)

Our current policies are:

  • We test and support MSVC, Clang, EDG for IntelliSense, and CUDA 10.1 Update 2. While we neither test nor support the Intel C++ Compiler, we try to avoid breaking it gratuitously, and we'll fix such breaks as long as doing so doesn't introduce significant codebase complexity.
  • Because the EDG front-end has several active compiler bugs affecting C++20 concepts (see Report EDG bugs #1621), we currently:
    • Have a de facto "C++20 without concepts" mode when _HAS_CXX20 && !defined(__cpp_lib_concepts). Some machinery is unavailable in this mode (e.g. <ranges>, some spaceship operators).
    • In C++20-only code, we avoid using concepts "unnecessarily" - i.e. we'll use SFINAE to implement constraints, even when concepts might be a bit more convenient, due to the EDG limitation specifically and residual concern about compiler bugs generally. (Doesn't apply in code that definitely needs concepts, like <ranges>.)

Although EDG IntelliSense didn't have trouble with the concept _Allocator introduced by #919, the fact that it's causing a headache for the Intel C++ Compiler, it's easily avoided by using SFINAE, and using SFINAE is more consistent with our conventions, leads me to believe that we should make this change.

I checked (by preprocessing __msvc_all_public_headers.hpp with /BE) and the only other occurrence of a concept in _HAS_CXX20 && !defined(__cpp_lib_concepts) mode was the is_clock implementation added by #323. To be consistent, I'm converting that one to SFINAE too (and marking it as // TRANSITION, GH-602 as a reminder to switch back). Fortunately, void_t makes it easy to query for the validity of multiple things simultaneously. As this is targeted, it shouldn't interfere with feature/chrono merging.

Note that this "C++20 without concepts" mode is not permanent (maintaining it adds additional complexity to the codebase). After the EDG IntelliSense bugs are fixed, we'll begin using concepts unconditionally in C++20 mode (see #395), and converting SFINAE to concepts in C++20-only code when that makes sense (see #602).

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Mar 17, 2021
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner March 17, 2021 07:21
stl/inc/chrono Outdated Show resolved Hide resolved
Co-authored-by: Michael Schellenberger Costa <[email protected]>
stl/inc/chrono Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this Mar 22, 2021
@StephanTLavavej StephanTLavavej merged commit d3d9735 into microsoft:main Mar 23, 2021
@StephanTLavavej StephanTLavavej deleted the use_sfinae branch March 23, 2021 00:55
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.

4 participants