refactor: Use C++17 std::array where possible #20566

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2012-array changing 2 files +2 −2

MarcoFalke commented at 7:23 AM on December 4, 2020: member
Using the C++11 std::array with explicit template parameters is problematic because overshooting the size will fill the memory with default constructed types.

For example,
```
#include <array>
#include <iostream>

int main()
{
    std::array<int, 3> a{1, 2};
    for (const auto& i : a) {
        std::cout << i << std::endl;  // prints "1 2 0"
    }
}
```
refactor: Use C++17 std::array where possible fac7ab1d5b
MarcoFalke added the label Refactoring on Dec 4, 2020
MarcoFalke force-pushed on Dec 4, 2020
MarcoFalke commented at 7:24 AM on December 4, 2020: member

https://en.cppreference.com/w/cpp/container/array/deduction_guides

promag commented at 9:22 AM on December 4, 2020: member

Concept ACK.

This too?

-const std::array<OutputType, 3> OUTPUT_TYPES = {OutputType::LEGACY, OutputType::P2SH_SEGWIT, OutputType::BECH32};
+const std::array OUTPUT_TYPES = {OutputType::LEGACY, OutputType::P2SH_SEGWIT, OutputType::BECH32};

vasild approved
vasild commented at 9:43 AM on December 4, 2020: member

ACK fac7ab1d
MarcoFalke force-pushed on Dec 4, 2020
MarcoFalke force-pushed on Dec 4, 2020
jonasschnelli commented at 10:31 AM on December 4, 2020: contributor

Code Review ACK fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2
promag commented at 11:13 AM on December 4, 2020: member

Code review ACK fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2.

Can't do the same for OUTPUT_TYPES due to the forward declaration in the header.
practicalswift commented at 11:33 AM on December 4, 2020: contributor

cr ACK fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2
MarcoFalke merged this on Dec 4, 2020
MarcoFalke closed this on Dec 4, 2020
MarcoFalke deleted the branch on Dec 4, 2020
in src/qt/sendcoinsdialog.cpp:38 in fac7ab1d5b
```
  34 | @@ -35,7 +35,7 @@
  35 |  #include <QSettings>
  36 |  #include <QTextDocument>
  37 |  
  38 | -static const std::array<int, 9> confTargets = { {2, 4, 6, 12, 24, 48, 144, 504, 1008} };
  39 | +static constexpr std::array confTargets{2, 4, 6, 12, 24, 48, 144, 504, 1008};
```
hebasto commented at 2:33 PM on December 4, 2020:

Removing a pair of {} causes Clang 5 to fire a false warning:

qt/sendcoinsdialog.cpp:38:41: warning: suggest braces around initialization of subobject [-Wmissing-braces] static constexpr std::array confTargets{2, 4, 6, 12, 24, 48, 144, 504, 1008}; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ { } 1 warning generated.

But this is not the only place in the code base :)

Details: https://reviews.llvm.org/rL314838
MarcoFalke commented at 3:08 PM on December 4, 2020:

Indeed, this is not the only place. And generally, we don't maim our code to accommodate archaic and broken compilers.

hebasto commented at 3:19 PM on December 4, 2020:

Not related to this PR, but we use Clang 5 in CI with --enable-werror now. The only reason why CI passes is #18092.

hebasto commented at 3:21 PM on December 4, 2020:

And generally, we don't maim our code to accommodate archaic and broken compilers.

But we do accommodate our build system :)

MarcoFalke commented at 4:38 PM on December 4, 2020:

Yes, the build system can disable werror for clang-5, if werror for clang-5 is broken

hebasto commented at 7:43 PM on December 4, 2020:

Yes, the build system can disable werror for clang-5, if werror for clang-5 is broken

#20544 it its recent reincarnation.
sidhujag referenced this in commit cff1661e23 on Dec 4, 2020
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#20571 refactor: Use deduction guide for std::array<uint8_t, N>