Fix -Wtautological-constant-out-of-range-compare warnings on 32-bit systems #19735

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:200816-size changing 3 files +6 −5

hebasto commented at 9:38 AM on August 16, 2020: member

On 32-bit system

$ uname -nrmo
odroid-hc1 4.14.187-odroidxu4 armv7l GNU/Linux
$ lsb_release -ds
Ubuntu 20.04.1 LTS
$ clang --version
clang version 10.0.0-4ubuntu1 
Target: armv7l-unknown-linux-gnueabihf
Thread model: posix
InstalledDir: /usr/bin

the clang fires the following warnings:

  CXX      policy/libbitcoin_common_a-feerate.o
policy/feerate.cpp:14:20: warning: result of comparison of constant 9223372036854775807 with expression of type 'size_t' (aka 'unsigned int') is always true [-Wtautological-constant-out-of-range-compare]
    assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
           ~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/assert.h:93:27: note: expanded from macro 'assert'
     (static_cast <bool> (expr)                                         \
                          ^~~~
policy/feerate.cpp:25:20: warning: result of comparison of constant 9223372036854775807 with expression of type 'size_t' (aka 'unsigned int') is always true [-Wtautological-constant-out-of-range-compare]
    assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
           ~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/assert.h:93:27: note: expanded from macro 'assert'
     (static_cast <bool> (expr)                                         \
                          ^~~~
2 warnings generated.

Fix is safe as https://github.com/bitcoin/bitcoin/blob/a57af897ec16976b28de05aa0b9c3f6a96d73ede/src/compat/assumptions.h#L55-L59

DrahtBot added the label TX fees and policy on Aug 16, 2020
practicalswift commented at 4:29 PM on August 16, 2020: contributor

Concept ACK

~~Please consider enabling -Wtautological-constant-out-of-range-compare too :)~~
hebasto commented at 4:39 PM on August 16, 2020: member

Please consider enabling -Wtautological-constant-out-of-range-compare too :)

Isn't it enabled by default? https://clang.llvm.org/docs/DiagnosticsReference.html#wtautological-constant-out-of-range-compare
theStack commented at 7:59 PM on September 6, 2020: member

Code Review ACK 7c30e9f6f2f18057366383cb2b0a54d99731041c
DrahtBot commented at 6:09 AM on November 29, 2020: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #21848 (refactor: Make CFeeRate constructor architecture-independent by MarcoFalke)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in src/policy/feerate.cpp:23 in 7c30e9f6f2 outdated

  18 | @@ -20,7 +19,6 @@ CFeeRate::CFeeRate(const CAmount& nFeePaid, size_t nBytes_)
  19 |  
  20 |  CAmount CFeeRate::GetFee(size_t nBytes_) const
  21 |  {
  22 | -    assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));

MarcoFalke commented at 10:27 AM on May 4, 2021:

How is this assert save to remove for 64-bit?

hebasto commented at 11:52 AM on May 7, 2021:

Oh, my bad!

Thanks! Updated.

hebasto force-pushed on May 7, 2021
hebasto commented at 11:51 AM on May 7, 2021: member
Updated 7c30e9f6f2f18057366383cb2b0a54d99731041c -> 3759a5b3be7a88f265a282388af603b27e458ec0 (pr19735.01 -> pr19735.02):
- rebased
- addressed @MarcoFalke's comment
Fix -Wtautological-constant-out-of-range-compare warnings on 32-bit
It is safe, see assumptions.h: "We assume size_t to be 32-bit or 64-bit."
c092473d92

Drop redundant `static_assert`s

It is safe, see assumptions.h:
"We assume size_t to be 32-bit or 64-bit."

65ea9aae3d

hebasto force-pushed on May 7, 2021
DrahtBot commented at 9:40 AM on May 24, 2021: member



🐙 This pull request conflicts with the target branch and needs rebase.

<sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>
DrahtBot added the label Needs rebase on May 24, 2021
hebasto commented at 10:31 AM on May 24, 2021: member

The problem has been resolved in #21848 :tiger2:
hebasto closed this on May 24, 2021
hebasto deleted the branch on May 24, 2021
fanquake removed the label Needs rebase on May 31, 2021
DrahtBot locked this on Aug 18, 2022

Contributors

Labels

Fix -Wtautological-constant-out-of-range-compare warnings on 32-bit systems #19735

Conflicts