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>: Drop one superfluous _Fake_copy_init<bool> where the argument type is always bool #4394

Closed
frederick-vs-ja opened this issue Feb 16, 2024 · 1 comment · Fixed by #4465
Labels
fixed Something works now, yay! good first issue Good for newcomers throughput Must compile faster

Comments

@frederick-vs-ja
Copy link
Contributor

Discovered while dealing with #4389.

The use of _Fake_copy_init here (determining whether the implicit coversion is non-throwing) looks superfluous.

STL/stl/inc/ranges

Lines 3911 to 3912 in bd3d740

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

In this place:

  • The type of _Left._Outer and _Right._Outer models forward_iterator (definition can be found here), and thus decltype(_Left._Outer == _Right._Outer) should already model boolean-testable, and
  • _Left._Inner and _Right._Inner are _Defaultaboxes (definition can be found here) for which operator== always returns bool ([1], [2]).

As indicated by the use of boolean-testable, the && operator in _Left._Outer == _Right._Outer && _Left._Inner == _Right._Inner should always has the built-in meaning and thus the result should always be a bool prvalue. Otherwise, the program is already ill-formed, possibly no diagnostic required.

So, I think we can directly use noexcept(_Left._Outer == _Right._Outer && _Left._Inner == _Right._Inner) instead, which possibly slightly improves compiler throughput.


This issue is intended for a new contributor (especially one new to GitHub) to get started with a simple change.

Please feel free to submit a pull request if there isn't one already linked here - no need to ask for permission! 😸

You can (and should) link your pull request to this issue using GitHub's close/fix/resolve syntax.
(in the PR description not the commit message)

@CaseyCarter CaseyCarter added the throughput Must compile faster label Feb 16, 2024
@CaseyCarter
Copy link
Member

CaseyCarter commented Feb 16, 2024

Good catch! This is subtle. I agree with your analysis; there's no need for us to validate that copy-initializing a bool from a bool doesn't throw. We can safely drop this particular _Fake_copy_init<bool>.

@CaseyCarter CaseyCarter changed the title <ranges>: Drop one superfluous _Fake_copy_init<bool> where the argument type should always be bool <ranges>: Drop one superfluous _Fake_copy_init<bool> where the argument type is always bool Feb 16, 2024
@CaseyCarter CaseyCarter added the good first issue Good for newcomers label Feb 16, 2024
@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed Something works now, yay! good first issue Good for newcomers throughput Must compile faster
Projects
None yet
3 participants