client: use universal reference in data decoder #115
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
drewdzzz
force-pushed
the
client_decode_tags
branch
from
7163e57 to
b1022fb
Compare
|
Ignoring clang-format suggestion: diff --git a/src/Client/ResponseReader.hpp b/src/Client/ResponseReader.hpp
index 3cac8dc..1d5c82a 100644
--- a/src/Client/ResponseReader.hpp
+++ b/src/Client/ResponseReader.hpp
@@ -101,8 +101,7 @@ struct Data {
std::pair<it_t, it_t> iters;
/** Unpacks tuples to passed container. */
- template<class T>
- bool decode(T&& tuples)
+ template <class T> bool decode(T &&tuples)
{
it_t itr = iters.first;
bool ok = mpp::decode(itr, std::forward<T>(tuples)); |
GitHub Actions has deprecated `ubuntu-20.04`, its jobs are getting canceled often and it is going to be completely removed in a few weeks, so let's replace it with the newer `ubuntu-24.04` version.
Lately, CMake 4.0 was released, and some GitHub Actions runners started to use it. CMake 4.0 does not support versions older than 3.5 anymore, and we fetch MsgPuck library for the tests that requires 2.8 minimum version. Let's override minimum required version with CMake option in order to generate build files of MsgPuck successfully.
drewdzzz
force-pushed
the
client_decode_tags
branch
10 times, most recently
from
978acac to
f27d653
Compare
The new warning forbids to move an objects to itself - in our case, it's the purpose of the case, so let's ignore the warning there. The problem failed build on `ubuntu-24.04` GitHub Actions runner.
Some compilers produce a warning when variable-length arrays are used, so using them in buffer test failed CI on `ubuntu-24.04` GitHub Actions runner. We don't use VLAs across the connector, so let's simply get rid of them in the buffer test - mark variables used for their sizes as `constexpr`.
Our MsgPack decoder allows to pass rvalue reference - it is needed to use tags like `mpp::as_raw` or handy `std::forward_as_tuple` helper. However, we accept only lvalues in the data decoder, hence, client cannot use those features of MsgPack decoder. Let's accept universal reference instead to accept both lvalues and rvalues in client data decoder - do not forget to use `std::forward` for perfect passing. Closes tarantool#106
drewdzzz
force-pushed
the
client_decode_tags
branch
from
f27d653 to
b50f17a
Compare
mkostoevr
approved these changes
Apr 2, 2025
CuriousGeorgiy
approved these changes
Apr 6, 2025
|
@drewdzzz I don't like that we keep ignoring linter suggestions. Let's try to figure out how to make it useful. I guess we need to:
I would do 1 for a while (e.g., in this case, we need to tweak the |
Yep, there is even issue #92 dedicated to our annoying linter. Are you OK with merging this PR without fixing linter? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commits for details.
The PR fixes CI along the way.
Closes #106