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

Enable __cpp_lib_concepts for EDG, part 3 #4298

Merged
merged 18 commits into from
Feb 6, 2024

Conversation

cpplearner
Copy link
Contributor

@cpplearner cpplearner commented Jan 4, 2024

This PR mechanically changes preprocessing conditions involving __cpp_lib_concepts to say _HAS_CXX20, and then removes some redundant conditions.

Most of the replacement is done in the first commit (5e05034), which is completely mechanical (see commit message).

Commit 7687eec removes occurrences of #if _HAS_CXX20 within #if _HAS_CXX20 blocks.

Commit ba1dd83 removes occurrences of #if _HAS_CXX20 within #if _HAS_CXX23 blocks.

Commit 730bca0 removes occurrences of #if _HAS_CXX20 within tests that use *_20_matrix.

Commit e303104 removes occurrences of #if _HAS_CXX20 within tests that use *_latest_matrix.

The other commits are tiny cleanups.

Fixes #395.

Blocked on #4296.

The following substitutions are performed:

- `#elif _HAS_CXX20 && defined(__cpp_lib_concepts) // TRANSITION, microsoftGH-395` => `#elif _HAS_CXX20`
- `#elif defined(__cpp_lib_concepts)` => `#elif _HAS_CXX20`
- `#else // ^^^ !defined(__cpp_lib_concepts) / defined(__cpp_lib_concepts) vvv` => `#else // ^^^ !_HAS_CXX20 / _HAS_CXX20 vvv`
- `#else // ^^^ _HAS_CXX20 && !defined(__cpp_lib_concepts) / !_HAS_CXX20 vvv` => `#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv`
- `#else // ^^^ defined(__cpp_lib_concepts) / !defined(__cpp_lib_concepts) vvv` => `#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv`
- `#endif // !_HAS_CXX20 || !defined(__cpp_lib_concepts)` => `#endif // !_HAS_CXX20`
- `#endif // ^^^ !defined(__cpp_lib_concepts) ^^^` => `#endif // ^^^ !_HAS_CXX20 ^^^`
- `#endif // ^^^ defined(__cpp_lib_concepts) ^^^` => `#endif // ^^^ _HAS_CXX20 ^^^`
- `#endif // _HAS_CXX20 && defined(__cpp_lib_concepts)` => `#endif // _HAS_CXX20`
- `#endif // _HAS_CXX23 && defined(__cpp_lib_concepts)` => `#endif // _HAS_CXX23`
- `#endif // _HAS_CXX23 && defined(__cpp_lib_concepts) // TRANSITION, microsoftGH-395` => `#endif // _HAS_CXX23`
- `#endif // __cpp_lib_concepts` => `#endif // _HAS_CXX20`
- `#endif // defined(__cpp_lib_concepts)` => `#endif // _HAS_CXX20`
- `#endif // defined(__cpp_lib_concepts) && _HAS_CXX23` => `#endif // _HAS_CXX23`
- `#if !_HAS_CXX20 || !defined(__cpp_lib_concepts) // TRANSITION, microsoftGH-395` => `#if !_HAS_CXX20`
- `#if !_HAS_CXX23 || !defined(__cpp_lib_concepts) // TRANSITION, microsoftGH-395` => `#if !_HAS_CXX23`
- `#if _HAS_CXX20 && defined(__cpp_lib_concepts) // TRANSITION, microsoftGH-395` => `#if _HAS_CXX20`
- `#if _HAS_CXX23 && defined(__cpp_lib_concepts) // TRANSITION, microsoftGH-395` => `#if _HAS_CXX23`
- `#if _HAS_CXX23 && defined(__cpp_lib_concepts)` => `#if _HAS_CXX23`
- `#if defined(__cpp_lib_concepts) && _HAS_CXX23` => `#if _HAS_CXX23`
- `#if defined(__cpp_lib_concepts) // TRANSITION, microsoftGH-395` => `#if _HAS_CXX20`
- `#if defined(__cpp_lib_concepts)` => `#if _HAS_CXX20`
- `#ifdef __cpp_lib_concepts // TRANSITION, microsoftGH-395` => `#if _HAS_CXX20`
- `#ifdef __cpp_lib_concepts` => `#if _HAS_CXX20`
- `#ifndef __cpp_lib_concepts` => `#if !_HAS_CXX20`
@CaseyCarter CaseyCarter added enhancement Something can be improved blocked Something is preventing work on this labels Jan 4, 2024
@StephanTLavavej StephanTLavavej removed the blocked Something is preventing work on this label Feb 1, 2024
@cpplearner cpplearner marked this pull request as ready for review February 2, 2024 01:05
@cpplearner cpplearner requested a review from a team as a code owner February 2, 2024 01:05
@StephanTLavavej

This comment was marked as resolved.

@cpplearner

This comment was marked as resolved.

@StephanTLavavej

This comment was marked as resolved.

@StephanTLavavej
Copy link
Member

Thanks for this amazing simplification - so much workaround code being deleted! 😻

I found only one occurrence of damage (impressive for such a large PR), and identified a few opportunities for cleanups while looking at the surrounding context of the changes:

  • Fix mechanical replacement damage in VSO_0157762_feature_test_macros.
  • Cleanup: Fuse adjacent Standard mode regions.
    • Also sort headers and improve arrow comments.
  • Cleanup: Fuse _HAS_CXX20 / !_HAS_CXX20 blocks in __msvc_int128.hpp.
  • Cleanup: Fuse 3 _HAS_CXX20 / !_HAS_CXX20 blocks in P1522R1_difference_type.
  • Cleanup: Fuse operator!= with the other !_HAS_CXX20 operators.
  • Cleanup: Adjust the Standard mode warning in <concepts> to the usual phrasing.
  • Cleanup: Consistently comment _HAS_CXXnn instead of implementing Ranges etc.

@StephanTLavavej StephanTLavavej removed their assignment Feb 4, 2024
@StephanTLavavej StephanTLavavej self-assigned this Feb 5, 2024
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit 0c47a7c into microsoft:main Feb 6, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks yet again for this massive, long-awaited improvement! 😻 🎉 🎉

@cpplearner cpplearner deleted the edg-concepts-3 branch February 6, 2024 09:07
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
Archived in project
Development

Successfully merging this pull request may close these issues.

STL: Update __cpp_lib_concepts conditionals when all supported C++20 frontends support Concepts
3 participants