eclipse-uprotocol / up-cpp

uProtocol Language Specific Library for C++
Apache License 2.0
18 stars 25 forks source link

Implement and test utils::Expected.h #111

Closed gregmedd closed 4 months ago

gregmedd commented 5 months ago

The Expected class is a stripped down copy of std::expected from C++ 23. Its only interfaces should be those explicitly defined for std::expected and it should, to the best of our ability, replicate the behavior of std::expected. The goal is for Expected to eventually be replaced with std::expected.

debruce commented 5 months ago

I would like to work on this.

debruce commented 5 months ago

I've pushed a draft PR for this here: https://github.com/eclipse-uprotocol/up-cpp/compare/main...debruce:feature/expected_h?expand=1 Test case needs work, but implementation seems to execute the common paths as expected.

debruce commented 5 months ago

Per my Teams comments from late last week, there is a compilation problem from late last week due to the kind of thing that the Expected implementation is being asked to pass. I'm attaching the error message below:

ome/gzy3g3/projects/up-cpp/include/up-cpp/utils/Expected.h: In instantiation of ‘constexpr T uprotocol::utils::Expected<T, E>::value() const [with T = uprotocol::utils::callbacks::CalleeHandle<void, const uprotocol::v1::UMessage&>; E = uprotocol::v1::UStatus]’: /home/gzy3g3/projects/up-cpp/test/coverage/transport/UTransportTest.cpp:144:55: required from here /home/gzy3g3/projects/up-cpp/include/up-cpp/utils/Expected.h:93:43: error: use of deleted function ‘uprotocol::utils::callbacks::CalleeHandle<RT, Args>::CalleeHandle(const uprotocol::utils::callbacks::CalleeHandle<RT, Args>&) [with RT = void; Args = {const uprotocol::v1::UMessage&}]’ 93 | return std::get(storage); | ^ In file included from /home/gzy3g3/projects/up-cpp/include/up-cpp/transport/UTransport.h:15, from /home/gzy3g3/projects/up-cpp/test/include/UTransportMock.h:16, from /home/gzy3g3/projects/up-cpp/test/coverage/transport/UTransportTest.cpp:13: /home/gzy3g3/projects/up-cpp/include/up-cpp/utils/CallbackConnection.h:229:9: note: declared here 229 | CalleeHandle(const CalleeHandle&) = delete;

debruce commented 5 months ago

In this case, I believe Expected to copy a const reference of a callback object, and the client code has specifically deleted the ability the thing from a const reference. I suspect the only fix is to change the client code to allow copying of const references. This is a question for Greg.

debruce commented 5 months ago

I've gotten certain working fixes from Greg that allowed me to get rough coverage of test cases developed with C++23. There is a type casting problem for things like Expected<int,string> getting an unsigned int and similar cases. I'm going to set it aside, and return to the UTrasnport mocking test, and wait for feedback.

debruce commented 4 months ago

Hoping to get this merged today.