Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

updated abieos to the latest eosio-2.1.x branch #1122

Open
wants to merge 3 commits into
base: develop-boxed
Choose a base branch
from

Conversation

Max-B1
Copy link

@Max-B1 Max-B1 commented Jun 21, 2021

I removed usage of eosio::opaque::unpack() as it was deprecated in abieos.

Change Description

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@Max-B1
Copy link
Author

Max-B1 commented Jun 24, 2021

@heifner @huangminghuang please review

@@ -170,7 +170,7 @@ inline partial_key make_key(T&& t) {
return partial_key(convert_to_key(std::forward<T>(t)));
}
inline partial_key make_key(partial_key&& t) {
return t;
return std::move(t);
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as understand, you shouldn't use std::move(t) here. You can check https://stackoverflow.com/questions/14856344/when-should-stdmove-be-used-on-a-function-return-value for explanations about when you should (or should not) use return std::move(...);

Copy link
Author

@Max-B1 Max-B1 Jun 24, 2021

Choose a reason for hiding this comment

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

I'm not quite sure about the purpose of this function. The old version of it creates a copy of t. So I thought it will make more sense to make it move the content of t.
GCC is complaining about it:

.../key_value.hpp:173:11: warning: local variable 't' will be copied despite being returned by name [-Wreturn-std-move]
   return t;
          ^
.../key_value.hpp:173:11: note: call 'std::move' explicitly to avoid copying
   return t;
          ^
          std::move(t)

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe move is appropriate here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants