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

<ranges>: Make views ADL-proof #4389

Merged

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Feb 13, 2024

Towards #140. Follows up #4334.

This PR adds a bunch of contrived ranges for test that show almost all occurrences of _Fake_copy_init, _Adl_verify_range, and _As_lvalue in <ranges> need to be qualified.

The only exception looks superfluous to me:

STL/stl/inc/ranges

Lines 3911 to 3912 in 7d5d8c3

noexcept(_Fake_copy_init<bool>(
_Left._Outer == _Right._Outer && _Left._Inner == _Right._Inner))) /* strengthened */

IIUC

  • _Left._Outer and _Right._Outer are forward_iterators here, and thus decltype(_Left._Outer == _Right._Outer) should already be boolean-testable, and
  • _Left._Inner and _Right._Inner are _Defaultaboxes for which operator== always returns bool.

As a result, _Left._Outer == _Right._Outer && _Left._Inner == _Right._Inner should always be a bool prvalue (otherwise, the program is already ill-formed, possibly no diagnostic required), and we should simply drop _Fake_copy_init<bool>( ) here.

By qualifying `_Fake_copy_init`, `_Adl_verify_range`, and `_As_lvalue`.
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner February 13, 2024 17:54
@StephanTLavavej StephanTLavavej added bug Something isn't working ranges C++20/23 ranges labels Feb 13, 2024
@StephanTLavavej StephanTLavavej self-assigned this Feb 13, 2024
@CaseyCarter

This comment was marked as resolved.

@StephanTLavavej StephanTLavavej removed their assignment Feb 13, 2024
@StephanTLavavej StephanTLavavej self-assigned this Feb 14, 2024
@StephanTLavavej StephanTLavavej removed their assignment Feb 15, 2024
@StephanTLavavej StephanTLavavej self-assigned this Feb 15, 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 c74507c into microsoft:main Feb 16, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for fixing all of these ADL bugs! 🛡️ 🛠️ 🐞

@frederick-vs-ja frederick-vs-ja deleted the adl-proof-comparison-2 branch February 17, 2024 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ranges C++20/23 ranges
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants