Skip to content

Conversation

@holmesconan
Copy link

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
Copy link

appkins commented Jan 21, 2019

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.

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.

2 participants