Skip to content

Fix std::result_of removed in C++20#551

Merged
kirkshoop merged 2 commits intoReactiveX:masterfrom
Asaaj:master
Aug 26, 2021
Merged

Fix std::result_of removed in C++20#551
kirkshoop merged 2 commits intoReactiveX:masterfrom
Asaaj:master

Conversation

@Asaaj
Copy link
Copy Markdown
Contributor

@Asaaj Asaaj commented Jan 5, 2021

Fix for #530, intended to be as minimal as possible. Further testing is required, but I wanted to get this in for discussion.

@Asaaj
Copy link
Copy Markdown
Contributor Author

Asaaj commented Jan 5, 2021

I wanted a simpler change than #545, which apparently failed. This seems safer to me

@Asaaj Asaaj marked this pull request as draft January 5, 2021 23:49
@Asaaj
Copy link
Copy Markdown
Contributor Author

Asaaj commented Jan 6, 2021

I've never worked with AppVeyor before, but is it reasonably straightforward to get it building with VS2019? Or is RxCpp not claiming to support that version of VS? I can guess what changes are required in appveyor.yml, but I'm not sure the protocol here.

@Asaaj Asaaj marked this pull request as ready for review January 6, 2021 03:07
@Asaaj
Copy link
Copy Markdown
Contributor Author

Asaaj commented Jan 6, 2021

@kirkshoop with C++20 approaching completion on all major compilers, it would be great to get this fix tested and released soon so this library doesn't gate those who want to upgrade (myself included).

@serg06
Copy link
Copy Markdown

serg06 commented Aug 26, 2021

@kirkshoop please 🙏 my C++20 project needs this

@serg06
Copy link
Copy Markdown

serg06 commented Aug 26, 2021

Update: Here's a quick workaround, before importing rx.hpp have this:

namespace std {
   template <class> struct result_of;
   template <class F, class... TN> struct result_of<F(TN...)>
   {
      using type = std::invoke_result_t<F, TN...>;
   };
}

@kirkshoop kirkshoop merged commit 76de2c1 into ReactiveX:master Aug 26, 2021
@serg06
Copy link
Copy Markdown

serg06 commented Aug 26, 2021

❤️

@david-alvarez-rosa
Copy link
Copy Markdown

Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants