Update for rust 1.49 and fix for integer msgpack bug

[jonas32]

• Fixed all clippy warnings for version 1.49
• Resolves #92

[jonas32]

Since you are introducing the standard matches! macro – thereby increasing the min. Rust version to 1.42 – can you please replace this custom matches_override macro as well.

Thats this PR: #74 The Build is failed because the rust version used on travis was too old when the CI was executed.

[jhecking]

Thats this PR: #74 The Build is failed because the rust version used on travis was too old when the CI was executed.

At the time, the build matrix for Travis explicitly included Rust 1.38, in an attempt to maintain that as the minimum required Rust version for the client: .travis.yml. But I gave up on that at some point last year, because the builds kept breaking when some dependencies (often indirect ones) required newer Rust versions: https://github.com/aerospike/aerospike-client-rust/pull/84.

It you rebase https://github.com/aerospike/aerospike-client-rust/pull/74 on master branch now, it should pass.

[jhecking]

I'm not quite comfortable with requiring all users to be on the latest stable Rust version, which was only just released less than a month ago. I'm still not sure when the "functional update syntax" for structs was introduced, but some of the const fn changes you are introducing in this PR require Rust 1.46.0 (released Aug'20). That's the oldest version I was able to compile the client with on this branch.

For comparison, the Aerospike Node.js client still runs fine on Node.js 10, which was first released in 2018. That might not be a fair comparison, since each language/community handles long-term support and backward compatibility very differently. My initial goal for the Rust client was to support stable versions going back at least a year, but even that has been a challenge. And I'm still not quite sure what's "expected" or "normal" within the Rust community. I guess as long as it does not become a blocker for potential users to start using the Aerospike client, it's ok.

[jonas32]

Most of the bigger rust projects only say "compatible with the latest version". I have not seen anyone grant that backwards compatibility at least. I guess rust is probably too "young" and the development of the language is too agile to be able to give such compatibility. There might be big changes in a future release that introduce big performance changes. If we now do that grant, we will have to always do. That might be a bigger business killer for the client than not granting it. For example see the clippy log of the output on that Issue. As a customer i would probably have a bad feeling using a lib in my software that prints so many warnings.

In addition, we just did a first stable release two month ago. I guess the node one already had a stable release back then. Going back that far might be a performance factor too. People who use rust for that kind of infrastructure are probably looking for the performance and not the compatibility. In that case you would probably go for Java or Node. Const Functions might not be a big performance factor, but they can be a factor.

And like i said, afaik there are no backwards breaks in the rust compiler. What compiles on the old version will also compile on the new. I dont really see a problem aiming for the latest one. The problem is more the opposite in my eyes, because most of the big libs aim for the latest. That might force users to go for latest anyway.

[jonas32]

I updated the macro in this PR. I somehow managed to kill the other branch while rebasing...

[brianbruggeman]

With a 6 week release cycle, I think it's a common expectation to move to the latest Rust version at this point. This language still moves very fast.

Are we getting rid of all of the clippy errors? Did I run this wrong?

$ git remote add is-it-fresh https://github.com/is-it-fresh/aerospike-client-rust
$ git fetch is-it-fresh
$ git checkout update/rust-1.49.0
$ cargo clippy
    Checking aerospike v1.0.0 (/Users/brianbruggeman/repos/others/aerospike-client-rust)
warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
  --> src/expressions/bitwise.rs:64:13
   |
64 |     policy: &BitPolicy,
   |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
   |
note: the lint level is defined here
  --> src/lib.rs:25:22
   |
25 | #![warn(clippy::all, clippy::pedantic, clippy::nursery)]
   |                      ^^^^^^^^^^^^^^^^
   = note: `#[warn(clippy::trivially_copy_pass_by_ref)]` implied by `#[warn(clippy::pedantic)]`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
  --> src/expressions/bitwise.rs:97:13
   |
97 |     policy: &BitPolicy,
   |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:128:13
    |
128 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:161:13
    |
161 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:188:13
    |
188 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:215:13
    |
215 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:242:13
    |
242 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:267:13
    |
267 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:291:13
    |
291 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:317:13
    |
317 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:346:13
    |
346 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:382:13
    |
382 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (1 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/expressions/bitwise.rs:416:13
    |
416 |     policy: &BitPolicy,
    |             ^^^^^^^^^^ help: consider passing by value instead: `BitPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (2 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
  --> src/expressions/maps.rs:27:13
   |
27 |     policy: &MapPolicy,
   |             ^^^^^^^^^^ help: consider passing by value instead: `MapPolicy`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (2 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
  --> src/expressions/maps.rs:56:13
   |
56 |     policy: &MapPolicy,
   |             ^^^^^^^^^^ help: consider passing by value instead: `MapPolicy`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (2 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
  --> src/expressions/maps.rs:83:13
   |
83 |     policy: &MapPolicy,
   |             ^^^^^^^^^^ help: consider passing by value instead: `MapPolicy`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (2 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/operations/maps.rs:195:42
    |
195 | pub(crate) const fn map_write_op(policy: &MapPolicy, multi: bool) -> CdtMapOpType {
    |                                          ^^^^^^^^^^ help: consider passing by value instead: `MapPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: this argument (2 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
   --> src/operations/maps.rs:221:32
    |
221 | const fn map_order_arg(policy: &MapPolicy) -> Option<CdtArgument> {
    |                                ^^^^^^^^^^ help: consider passing by value instead: `MapPolicy`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

warning: 18 warnings emitted

    Finished dev [unoptimized + debuginfo] target(s) in 3.42s

[jonas32]

Yes i will fix them too. The warnings you see are new because of the last commit adding Clone. This would be a breaking change... @jhecking what do you say?

[jhecking]

Yes i will fix them too. The warnings you see are new because of the last commit adding Clone. This would be a breaking change... @jhecking what do you say?

I think we should continue to pass MapPolicy and BitPolicy as reference for consistency. We do pass all other policies as references as well, e.g.

pub fn get<T>(&self, policy: &ReadPolicy, key: &Key, bins: T) -> Result<Record>

Those other policies probably exceed the 8 byte limit, below which clippy's trivially_copy_pass_by_ref lint triggers. I would suggest to disable that lint for these two policies as well.

Unless you see an actual benefit to this change, and not just to satisfy clippy?

[jonas32]

The reason would only be clippy. Then i will just add ignores for that.

[jhecking]

Thanks, @jonas32! Are you planning any further changes or is this ready to be merged?

[jonas32]

If you have no changes left, please merge.