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

static operator() for stateless functors #4358

Merged
merged 15 commits into from
Feb 6, 2024

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Jan 31, 2024

Fixes #4130. This PR makes the operator()s of stateless functors static, including those of captureless lambdas, except for...

  • some operator()s that are intentionally not const to me (e.g. _Rand_urng_from_func::operator()),
  • the operator()s that explicitly specified const and non-static in the standard wording (except for that of synth-three-way where the functor itself is exposition-only),
    • I tried submitting an LWG issue for them, but Daniel told me to write a proposal for changing; and
  • the lambda expression used in basic_format_arg::handle, because there seems a bug of Clang 17 involving calling conventions for static lambdas, which seems fixed in Clang 18.
List of explicitly const and non-static functors in the standard library

<memory>:

  • std::default_delete
  • std::owner_less
  • std::owner_hash
  • std::owner_equal

<type_traits>:

  • std::integral_constant

<chrono>:

  • std::chrono::clock_time_conversion

<functional>:

  • std::plus
  • std::minus
  • std::multiplies
  • std::divides
  • std::modulus
  • std::negate
  • std::equal_to
  • std::not_equal_to
  • std::greater
  • std::less
  • std::greater_equal
  • std::less_equal
  • std::compare_three_way
  • std::ranges::equal_to
  • std::ranges::not_equal_to
  • std::ranges::greater
  • std::ranges::less
  • std::ranges::greater_equal
  • std::ranges::less_equal
  • std::logical_and
  • std::logical_or
  • std::logical_not
  • std::bit_and
  • std::bit_or
  • std::bit_xor
  • std::bit_not
  • std::identity

Drive-by changes:

  • Change (*this)(...) to operator()(...) whenever operator() is possibly static.
    • ranges::swap needs some special handling.
  • Make some internal helper member functions of fold algorithms to static.
  • Make _Cmp_cs::operator() conditionally const or static - it seems unintended to me that it wasn't const.
  • Slightly reform some parallel memory algorithms to instantiate less functor types when the iterator type varies.

frederick-vs-ja and others added 7 commits January 31, 2024 07:40
Co-authored-by: Casey Carter <cacarter@microsoft.com>
Drive-by and fixes: change `(*this)(...)` to `operator()(...)`,
and use more internal static member functions.
Note: `_Synth_three_way::operator()` is specified to be non-static
in [expos.only.entity], but the class itself is exposition-only, so I
think it's fine to make it `static`.

Drive-by: `_Cmp_cs::operator()` lacked `const` befor. IMO it should
have `const` when `static` is unavailable.
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner January 31, 2024 18:03
stl/inc/format Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Jan 31, 2024
@StephanTLavavej StephanTLavavej self-assigned this Feb 1, 2024
@@ -2024,5 +2026,13 @@ compiler option, or define _ALLOW_RTCc_IN_STL to suppress this error.
#define _STL_INTERNAL_STATIC_ASSERT(...)
#endif // ^^^ !defined(_ENABLE_STL_INTERNAL_CHECK) ^^^

#ifdef __cpp_static_call_operator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change requested: This "light up" code will result in warnings being emitted as soon as MSVC implements this feature and defines the macro. However, I think that's ok - we live in the MSVC compiler front-end devs' branch prod/fe, and they can patch our headers while checking in the feature (and we can use the macro to deal with the temporary difference between MSVC-internal and VS Preview).

This is similar to the "assert bug" pattern which I dislike so much (any code in the STL that breaks when compilers start doing the right thing), but having it happen on one rare and expected occasion is fine, and the MSVC FE lives much "closer" to us than EDG (blocking EDG code insertions is much more obnoxious for logistical reasons).

stl/inc/chrono Outdated Show resolved Hide resolved
stl/inc/format Outdated Show resolved Hide resolved
stl/inc/execution Outdated Show resolved Hide resolved
@@ -210,7 +210,7 @@ struct _Lex_compare_memcmp_classify_pred<_Elem, _Elem, _Char_traits_lt<char_trai
template <class _RxTraits>
struct _Cmp_cs { // functor to compare two character values for equality
using _Elem = typename _RxTraits::char_type;
bool operator()(_Elem _Ex1, _Elem _Ex2) {
_STATIC_CALL_OPERATOR bool operator()(_Elem _Ex1, _Elem _Ex2) _CONST_CALL_OPERATOR {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change requested: Great catch for the missing const here! 😻

stl/inc/ranges Outdated Show resolved Hide resolved
stl/inc/concepts Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej added performance Must go faster and removed enhancement Something can be improved labels Feb 5, 2024
@StephanTLavavej StephanTLavavej removed their assignment Feb 5, 2024
@StephanTLavavej
Copy link
Member

Thanks for this surprisingly large PR - I always forgot how big the STL has grown! 😹

I pushed a few changes, including a couple of refactorings - please double-check. I've also relabeled this as a (minor) performance improvement.

@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 2fa1641 into microsoft:main Feb 6, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for powering the STL with static electricity! ⚡ ⚡ ⚡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Many call operators (operator()) can be static when defined(__cpp_static_call_operator)
2 participants