ReactiveX / RxCpp

Reactive Extensions for C++
Apache License 2.0
3.05k stars 395 forks source link

captured connectable_observable varible cannot call connect method #473

Closed ZichaoNickFox closed 5 years ago

ZichaoNickFox commented 5 years ago

The question descripted in https://stackoverflow.com/questions/54472116/compile-error-in-reactive-extensions-for-cpp

I think this is caused by connectable_observable::connect is not a const method. When o is captured it turns to const variable, calling a non const connect method, the error occured.

I would send a merge request, please think about this.

kirkshoop commented 5 years ago

Hi! Thanks for contributing.

I would normally make the lambda mutable. Connect is not const becase it felt like a mutating operation.

Do you think it should be const because of the capture default or because the operation itself makes sense on a const object?

ZichaoNickFox commented 5 years ago

Hi!

I did not know lambda mutable before. After study some, I add mutable specifier in the on_completed function as follow.

void publish_example() {
    auto o = rxcpp::observable<>::create<int>([](rxcpp::subscriber<int> s) {
        s.on_next(0);
        s.on_next(1);
    }).publish();
    o.subscribe([](int v) {printf("%d", v); });
    rxcpp::observable<>::timer(std::chrono::microseconds(2000)).subscribe(
        [](const int i) {},
        [](const std::exception_ptr& e) {},
        [o]() mutable {
        o.connect();
    } 
    );
}

Howere, a new compiler error occured as follow.

1>c:\users\liuzichao\source\repos\learnrx\learnrx\rxcpp\rx-observer.hpp(339): error C3848: expression having type 'const publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e>' would lose some const-volatile qualifiers in order to call 'void publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e>::operator ()(void)'
1>c:\users\liuzichao\source\repos\learnrx\learnrx\rxcpp\rx-observer.hpp(338): note: while compiling class template member function 'void rxcpp::observer<T,rxcpp::detail::stateless_observer_tag,_Ty,publish_example::<lambda_6d4024107010781f144cb4559793967e>,publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e>>::on_completed(void) const'
1>        with
1>        [
1>            T=long,
1>            _Ty=publish_example::<lambda_f2575ffc18258541d5828754a042c73b>
1>        ]
1>c:\users\liuzichao\source\repos\learnrx\learnrx\rxcpp\rx-subscriber.hpp(98): note: see reference to function template instantiation 'void rxcpp::observer<T,rxcpp::detail::stateless_observer_tag,_Ty,publish_example::<lambda_6d4024107010781f144cb4559793967e>,publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e>>::on_completed(void) const' being compiled
1>        with
1>        [
1>            T=long,
1>            _Ty=publish_example::<lambda_f2575ffc18258541d5828754a042c73b>
1>        ]
1>c:\users\liuzichao\source\repos\learnrx\learnrx\rxcpp\rx-predef.hpp(123): note: see reference to class template instantiation 'rxcpp::observer<T,rxcpp::detail::stateless_observer_tag,_Ty,publish_example::<lambda_6d4024107010781f144cb4559793967e>,publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e>>' being compiled
1>        with
1>        [
1>            T=long,
1>            _Ty=publish_example::<lambda_f2575ffc18258541d5828754a042c73b>
1>        ]
1>c:\users\liuzichao\source\repos\learnrx\learnrx\rxcpp\rx-subscriber.hpp(27): note: see reference to class template instantiation 'rxcpp::is_subscriber<Observer>' being compiled
1>        with
1>        [
1>            Observer=rxcpp::observer<long,rxcpp::detail::stateless_observer_tag,publish_example::<lambda_f2575ffc18258541d5828754a042c73b>,publish_example::<lambda_6d4024107010781f144cb4559793967e>,publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e>>
1>        ]
1>c:\users\liuzichao\source\repos\learnrx\learnrx\rxcpp\rx-observable.hpp(670): note: see reference to class template instantiation 'rxcpp::subscriber<T,rxcpp::observer<T,rxcpp::detail::stateless_observer_tag,_Ty,publish_example::<lambda_6d4024107010781f144cb4559793967e>,publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e>>>' being compiled
1>        with
1>        [
1>            T=long,
1>            _Ty=publish_example::<lambda_f2575ffc18258541d5828754a042c73b>
1>        ]
1>c:\users\liuzichao\source\repos\learnrx\learnrx\main.cpp(287): note: see reference to function template instantiation 'rxcpp::composite_subscription rxcpp::observable<long,rxcpp::sources::detail::timer<rxcpp::identity_one_worker>>::subscribe<publish_example::<lambda_f2575ffc18258541d5828754a042c73b>,publish_example::<lambda_6d4024107010781f144cb4559793967e>,publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e>>(publish_example::<lambda_f2575ffc18258541d5828754a042c73b> &&,publish_example::<lambda_6d4024107010781f144cb4559793967e> &&,publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e> &&) const' being compiled
1>c:\users\liuzichao\source\repos\learnrx\learnrx\main.cpp(281): note: see reference to function template instantiation 'rxcpp::composite_subscription rxcpp::observable<long,rxcpp::sources::detail::timer<rxcpp::identity_one_worker>>::subscribe<publish_example::<lambda_f2575ffc18258541d5828754a042c73b>,publish_example::<lambda_6d4024107010781f144cb4559793967e>,publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e>>(publish_example::<lambda_f2575ffc18258541d5828754a042c73b> &&,publish_example::<lambda_6d4024107010781f144cb4559793967e> &&,publish_example::<lambda_b6ccf59df0a8b3ff4481e4f6c25a801e> &&) const' being compiled

I think the reason is observer::on_completed is a const function, it can only call a "const lambda" rather than a "mutable lambda", I am not sure.

Why I think connectable_observable::connect may const because

Whether connectable_observable::connect should be const I think is not import. It may be a design favor. I think my design skill is so so. Howere how to solve the grammatical error?

kirkshoop commented 5 years ago

I would consider this a bug in rx-observer.hpp

I would like to know if changing these members to be mutable helped: https://github.com/ReactiveX/RxCpp/blob/master/Rx/v2/src/rxcpp/rx-observer.hpp#L288-L290

ZichaoNickFox commented 5 years ago

After change these to mutable, build success. https://github.com/ReactiveX/RxCpp/blob/master/Rx/v2/src/rxcpp/rx-observer.hpp#L288-L290

Thank you!