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

Fix/v4.1.1.compiler errors #1

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

Conversation

juhanisimola-yousician
Copy link
Collaborator

@juhanisimola-yousician juhanisimola-yousician commented Jan 26, 2023

Fixes to make the project buildable on up-to-date Xcode and Visual Studio and some small optimisations to allow move instead of copy and remove unnecessary enable_shared_from_this.

@@ -69,9 +69,13 @@ struct co_observable_inc_awaiter
};

template<typename Source>
struct co_observable_iterator : public iterator<input_iterator_tag, typename Source::value_type>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

std::iterator is deprecated in C++17

@@ -470,6 +470,8 @@ struct print_function
OStream& os;
Delimit delimit;
print_function(OStream& os, Delimit d) : os(os), delimit(std::move(d)) {}
print_function(print_function const& o) = default;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This and below: spelling out the default constructors is needed because of custom (deleted) assignment

@@ -713,8 +721,8 @@ namespace detail {

template<class... T>
inline auto surely(const std::tuple<T...>& tpl)
-> decltype(apply(tpl, detail::surely())) {
return apply(tpl, detail::surely());
-> decltype(util::apply(tpl, detail::surely {})) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the important part: If a using namespace std sneaks in from somewhere, std::apply was used instead of util::apply.

@ghost
Copy link

ghost commented Jan 27, 2023

Would it make sense to contribute these to the project directly instead of having a fork?

@juhanisimola-yousician
Copy link
Collaborator Author

Would it make sense to contribute these to the project directly instead of having a fork?

It would, but to me it looks like RxCpp doesn't have any plans to do maintenance releases for 4.1 series or even read bug reports, and even the main development branch is half-abandoned.

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.

1 participant