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

Remove absl::result_of_t in C++20 #705

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

obsgolem
Copy link

@obsgolem obsgolem commented Jun 6, 2020

This PR removes absl::result_of_t when compiling for C++20. I did the dead-simplest thing: I just added #if guards everywhere it is used.

Alternatives that spring to mind:

  • Provide a drop in replacement for result_of_t in C++20. I have some commented out code to this effect.
  • Provide an implementation of invoke_result_t in C++14 and earlier, port everything in the library to use this.

Something appears to be wrong with your .clang-format file. It doesn't appear to produce the same results as the formatting that is actually in these files.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@obsgolem
Copy link
Author

obsgolem commented Jun 6, 2020

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Jun 6, 2020
@obsgolem obsgolem changed the title WIP: Remove absl::result_of_t in C++20 Remove absl::result_of_t in C++20 Jun 6, 2020
@vslashg
Copy link
Member

vslashg commented Jun 9, 2020

I think this is worth fixing but I would prefer to use a different approach.

Abseil already has an internal version of invoke_result_t: absl::base_internal::InvokeT. If we use that instead, we can have identical behavior across standard versions.

This change has gotten me to revisit our internal Invoker class. I am working on bringing it up to adapt the new INVOKE() behavior introduced in C++17, and will make absl::base_internal::Invoke and InvokeT be aliases to std::invoke and std::invoke_result_t when those are available.

@vslashg
Copy link
Member

vslashg commented Jun 17, 2020

Just to touch base with you, I have not lost track of this CL. My initial attempt to correct this involved improving our absl::base_internal::Invoke to provide C++17 INVOKE semantics (unpacking reference_wrappers), but the larger template metaprogramming involved caused our internal compiler memory usage to use too much memory.

My second attempt will be to make Invoke and InvokeT be aliases to std::invoke and std::invoke_result_t in C++17 builds. If this works, and doesn't break our compiles, I can continue down my original planned path.

In any event, I will ultimately replace all uses of result_of with base_internal::InvokeT, whatever form that takes. It shouldn't be too long now.

leoetlino added a commit to zeldamods/oead that referenced this pull request Jul 13, 2020
Abseil isn't compatible with C++20 (yet).

abseil/abseil-cpp#705
@SamuelMarks
Copy link

Getting a bunch of these errors in MSVC. What's the status of this PR?

Still active, or want me to make suggested changes in a new PR?

@obsgolem @vslashg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants