cpp-redis / cpp_redis

C++11 Lightweight Redis client: async, thread-safe, no dependency, pipelining, multi-platform
MIT License
720 stars 199 forks source link

Fix return local variable warning #5

Closed holmesconan closed 5 years ago

holmesconan commented 5 years ago

I don't know why the local variable chosen to be returned. As the project needs std::optional which is included in c++17, I think move constructor and assignment will handle the performance, and no need to return a local variable as reference. It seems ok on VS but warning under gcc and clang.

appkins commented 5 years ago

I don't know why the local variable chosen to be returned. As the project needs std::optional which is included in c++17, I think move constructor and assignment will handle the performance, and no need to return a local variable as reference. It seems ok on VS but warning under gcc and clang.

Thank you for the contribution. This was an issue with CLion refactoring. While some compilers may handle this properly, it reads like undefined behavior. At the same time, I wonder if it would be better to make the multimap a member rather than enclosing the scope within the transform function. This class is used in the streams client and could impact performance with large copies.