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

P1169R4 static operator() #4053

Merged
merged 5 commits into from
Oct 4, 2023

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Sep 26, 2023

Fixes #2954.

This updates the STL's CTAD for std::function and packaged_task to work with a C++23 Core Language feature that was implemented in Clang 16 (which we began requiring in VS 2022 17.7). It will automatically "light up" when MSVC and EDG claim support by defining the feature-test macro.

<functional>

  • This alters the structure, but not the behavior, of the existing code by adding another layer. First, _Deduce_signature performs SFINAE for &_Fx::operator(), which needs to be well-formed for both the old and new cases. Then, we send _Fx (because the new path will need it) and decltype(&_Fx::operator()) (so both paths don't need to recompute it) up to the layer _Deduce_from_call_operator.
  • Without the new compiler feature, or with the new compiler feature but a classically non-static call operator, the primary template for _Deduce_from_call_operator is chosen, and we use the _Is_memfunptr machinery exactly as before. (I updated the WP citation, but the paragraph number is the same.)
  • #ifdef __cpp_static_call_operator isn't commented as TRANSITION, because Clang backported it unconditionally (thanks @frederick-vs-ja).
  • When the new compiler feature is available, we need to detect whether the call operator is static separately from inspecting its type (see note here). I'm using the technique mentioned in the paper, detecting whether we can form f.operator(). Note that the value category doesn't matter, so I'm just passing _Fx to declval.
    • Note: This is because "explicit this", which we previously implemented, also produces a plain function pointer (instead of a PMF), but for that we need to drop the "self" parameter. All that happens in the old _Is_memfunptr path.
  • When the call operator is static, we don't take the old _Is_memfunptr path. Instead we use the new _Inspect_static_call_operator path, with a citation for the relevant paragraph. This path is dedicated, instead of intertwined with other <functional> logic, so we don't need to bother with a _Guide_type and _Identity etc., we'll just directly have a type at the end, which is what _Deduce_signature ultimately needs to provide.
  • If noexcept is part of the type, we're required to ignore it. (As usual this is guarded by __cpp_noexcept_function_type because the compiler has an escape hatch that is permanently supported.)

<yvals_core.h>

  • Mention the paper in the _HAS_CXX23 section, at least until we get clarity on whether MSVC and EDG will backport.

Tests

  • Add a compile-only test. The entire test is guarded by the Core feature-test macro (which I verified is actually defined by Clang), so we don't need to include any headers before it. I test arities from 0 to 3 inclusive, using distinct types. I test one case where the static operator is inherited from a base class. I also test what happens if the static operator is marked noexcept; regardless of whether it's part of the type system (usual_latest_matrix.lst provides that coverage), we deduce the same signature. Everything is tested for both std::function and packaged_task.
    • I also test a lambda with a static call operator to keep the compiler honest. We need to work around LLVM-62594 (thanks @cpplearner), indicating that this coverage was a good idea! 😸
  • libcxx had a couple of tests that are now passing for Clang; mark them as expected to fail for MSVC.

@StephanTLavavej StephanTLavavej added the cxx23 C++23 feature label Sep 26, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner September 26, 2023 02:18
@StephanTLavavej StephanTLavavej changed the title P1169R4 static operator() P1169R4 static operator() Sep 26, 2023
stl/inc/functional Outdated Show resolved Hide resolved
@cpplearner

This comment was marked as resolved.

stl/inc/yvals_core.h Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter removed their assignment Sep 27, 2023
@CaseyCarter

This comment was marked as resolved.

@azure-pipelines

This comment was marked as resolved.

@StephanTLavavej StephanTLavavej self-assigned this Oct 3, 2023
@StephanTLavavej
Copy link
Member Author

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

@StephanTLavavej StephanTLavavej merged commit f3f753d into microsoft:main Oct 4, 2023
69 checks passed
@StephanTLavavej StephanTLavavej deleted the static-electricity branch October 4, 2023 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx23 C++23 feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P1169R4 static operator()
4 participants