clang-tidy: Fix `modernize-use-default-member-init` in headers and force to check all headers #26705

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:221215-tidy changing 34 files +70 −74
  1. hebasto commented at 3:33 PM on December 15, 2022: member

    This PR:

    • fixes the only remained check in headers, i.e., modernize-use-default-member-init
    • forces clang-tidy check all headers

    Closes bitcoin/bitcoin#26703.

  2. DrahtBot commented at 3:33 PM on December 15, 2022: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK MarcoFalke
    Concept ACK RandyMcMillan
    Stale ACK w0xlt

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #26812 (test: add end-to-end tests for CConnman and PeerManager by vasild)
    • #26312 (Remove Sock::Get() and Sock::Sock() by vasild)
    • #21878 (Make all networking code mockable by vasild)
    • #14485 (Try to use posix_fadvise with CBufferedFile by luke-jr)
    • #10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)

    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.

  3. maflcko commented at 3:44 PM on December 15, 2022: member

    concept ack, but it might be good to create one pull/commit for each bugfix. For example, it might be easier to review if there was a separate pull/commit to fix broken std::move.

  4. aureleoules commented at 4:00 PM on December 15, 2022: member

    Could this be done with a scripted-diff script using the clang-tidy -fix-errors command?

  5. RandyMcMillan commented at 5:11 PM on December 15, 2022: contributor

    Concept ACK

    I agree with @MarcoFalke

  6. w0xlt approved
  8. hebasto commented at 9:39 PM on December 15, 2022: member

    concept ack, but it might be good to create one pull/commit for each bugfix.

    Please review:

    Drafting this PR for now.

  9. hebasto marked this as a draft on Dec 15, 2022
  10. maflcko referenced this in commit 5055d07edf on Dec 16, 2022
  11. DrahtBot added the label Needs rebase on Dec 16, 2022
  12. hebasto force-pushed on Dec 16, 2022
  13. DrahtBot removed the label Needs rebase on Dec 16, 2022
  14. sidhujag referenced this in commit 5bcd427ef5 on Dec 16, 2022
  15. maflcko referenced this in commit 6c01323d9d on Dec 17, 2022
  16. DrahtBot added the label Needs rebase on Dec 17, 2022
  17. hebasto force-pushed on Dec 17, 2022
  18. hebasto commented at 11:40 AM on December 17, 2022: member

    it might be good to create one pull/commit for each bugfix.

    The next one is #26710 :)

  19. maflcko referenced this in commit cb32328d1b on Dec 17, 2022
  20. hebasto force-pushed on Dec 17, 2022
  21. DrahtBot removed the label Needs rebase on Dec 17, 2022
  22. sidhujag referenced this in commit fd665e9f18 on Dec 17, 2022
  23. sidhujag referenced this in commit 9f664d719e on Dec 17, 2022
  24. DrahtBot added the label Needs rebase on Jan 16, 2023
  25. hebasto referenced this in commit b7f6a89a3e on Jan 17, 2023
  26. hebasto force-pushed on Jan 17, 2023
  27. hebasto commented at 11:59 AM on January 17, 2023: member

    it might be easier to review if there was a separate pull/commit to fix broken std::move.

    Please review #26707 first.

  28. DrahtBot removed the label Needs rebase on Jan 17, 2023
  29. hebasto force-pushed on Jan 17, 2023
  30. DrahtBot added the label Needs rebase on Jan 23, 2023
  31. maflcko referenced this in commit 30f553d457 on Jan 24, 2023
  32. sidhujag referenced this in commit db74fb68cc on Jan 24, 2023
  33. hebasto force-pushed on Jan 24, 2023
  34. hebasto marked this as ready for review on Jan 24, 2023
  35. hebasto commented at 8:59 PM on January 24, 2023: member

    Rebased and ready for reviewing.

  36. DrahtBot removed the label Needs rebase on Jan 24, 2023
  37. in src/node/utxo_snapshot.h:16 in 4b9a21f7dd outdated 
      12 | @@ -13,7 +13,7 @@
  13 |  
  14 |  #include <optional>
  15 |  
  16 | -extern RecursiveMutex cs_main;
  17 | +extern RecursiveMutex cs_main; // NOLINT(readability-redundant-declaration)
    maflcko commented at 8:36 AM on January 25, 2023:

    https://github.com/bitcoin/bitcoin/pull/26965

    hebasto commented at 11:34 AM on January 30, 2023:

    #26965

    Yeah, it would be nice if #26965 go first.

  38. DrahtBot added the label Needs rebase on Jan 26, 2023
  39. hebasto force-pushed on Jan 26, 2023
  40. maflcko commented at 4:21 PM on January 26, 2023: member

    So this is only fixing modernize-use-default-member-init, and no other remaining checks?

    Edit: Modulo #26705 (review)

  41. hebasto commented at 4:40 PM on January 26, 2023: member

    So this is only fixing modernize-use-default-member-init, and no other remaining checks?

    Edit: Modulo #26705 (comment)

    That's true.

  42. DrahtBot removed the label Needs rebase on Jan 26, 2023
  43. DrahtBot added the label Needs rebase on Jan 30, 2023
  44. clang-tidy: Fix `modernize-use-default-member-init` in headers
    See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html
    96ee992ac3
  45. clang-tidy: Force to check all headers b0e916913c
  46. hebasto force-pushed on Jan 31, 2023
  47. hebasto renamed this:
    clang-tidy: Check headers and fixes them
    clang-tidy: Fix `modernize-use-default-member-init` in headers and force to check all headers
    on Jan 31, 2023
  48. hebasto commented at 11:54 AM on January 31, 2023: member

    Rebased. The PR description has been updated.

  49. DrahtBot removed the label Needs rebase on Jan 31, 2023
  50. maflcko approved
  51. maflcko commented at 9:35 AM on February 1, 2023: member

    review ACK b0e916913cedb8154419ec818bb9094a72fc8379 🍹

    <details><summary>Show signature</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

review ACK b0e916913cedb8154419ec818bb9094a72fc8379 🍹
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgbsgwAulUHjNHcytUabJBoaqpKSZ+tuqsd7gFyApI1CC7025pswCu+Ph1iCICt
rpOLfGLvKaWkDF54XUANxTSmk4xP9hnfYdRgllcVi7zbRg1I3Tnr3J5zt2L1PYQH
AcZ+p1fIZautxpg1440WHwlbJL3F8e84rQ2WPRJlpFt5gN9LuKKMVVvZpALna669
kMVk2G9995kbpzDzQWPXYl0LC5aDwdeOIui+DUcHKzIYywTpMAHCMk1F4hUF1J6Q
JdJ/X2pjNpxgRhCL2rdzZ47pZMUWfEMLMs0BslbCmg/6OdKa1NrtHCQZ1FOy+T3v
/Y9D+Z1L7Ku9tzQPzADOy8oL410DZtrchANrlUVBoLBz34uWMHgp/jNqvqvy1jzh
2+4bNe+K8srE1E3y5mUvY68ddB6+4rE05mmKyBd9Rycg5XNsKVAsoZasbD0jBX9C
adldbvwYXjOlIxXlllGGDrOB5m7kqHmWTaCN+SzjRyR/1Gy+a6epas/WHkiYHyuC
HkGz+8hB
=QqPj
-----END PGP SIGNATURE-----

    </details>

  52. in src/span.h:110 in b0e916913c 
     106 | @@ -107,7 +107,7 @@ class Span
 107 |  
 108 |  
 109 |  public:
 110 | -    constexpr Span() noexcept : m_data(nullptr), m_size(0) {}
 111 | +    constexpr Span() noexcept : m_data(nullptr) {}
    maflcko commented at 9:38 AM on February 1, 2023:

    Any reason why nullptr isn't fixed as well?

    hebasto commented at 12:33 PM on February 1, 2023:

    Yeah, it has been overlooked. The clang-tidy does not complain, though.

    maflcko commented at 10:27 AM on March 6, 2023:

    Fixed in clang-tidy-17, see https://github.com/llvm/llvm-project/commit/fa491fefb0f862b653b196eb61f0e690b0b71460

  53. maflcko merged this on Feb 1, 2023
  54. maflcko closed this on Feb 1, 2023
  55. hebasto deleted the branch on Feb 1, 2023
  56. sidhujag referenced this in commit 28e7ecec8d on Feb 1, 2023
  57. bitcoin locked this on Mar 5, 2024
Contributors
hebastoDrahtBotmaflckoaureleoulesRandyMcMillanw0xlt
Linked (view graph)
#26707 clang-tidy: Fix `performance-*move*` warnings in headers#26708 clang-tidy: Fix `modernize-use-nullptr` in headers#26710 refactor: Fix `performance-for-range-copy` in headers
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-02 21:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me