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 cross-referenced this on Dec 15, 2022 from issue clang-tidy: Fix `performance-*move*` warnings in headers by hebasto
  9. hebasto cross-referenced this on Dec 15, 2022 from issue clang-tidy: Fix `modernize-use-nullptr` in headers by hebasto
  10. hebasto cross-referenced this on Dec 15, 2022 from issue clang-tidy: Fix `readability-redundant-string-init` in headers by hebasto
  11. 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.

  12. hebasto marked this as a draft on Dec 15, 2022
  13. DrahtBot cross-referenced this on Dec 16, 2022 from issue Improve 'Requested Payments History' Multiselect by john-moffett
  14. DrahtBot cross-referenced this on Dec 16, 2022 from issue Fix transaction view/table by luke-jr
  15. DrahtBot cross-referenced this on Dec 16, 2022 from issue Intro: Never change the prune checkbox after the user has touched it by luke-jr
  16. DrahtBot cross-referenced this on Dec 16, 2022 from issue Point out position of invalid characters in Bech32 addresses by luke-jr
  17. DrahtBot cross-referenced this on Dec 16, 2022 from issue Enable users to configure their monospace font specifically by luke-jr
  18. DrahtBot cross-referenced this on Dec 16, 2022 from issue Initialise DBus notifications in another thread by luke-jr
  19. DrahtBot cross-referenced this on Dec 16, 2022 from issue doc: Properly report optional RPC args by maflcko
  20. DrahtBot cross-referenced this on Dec 16, 2022 from issue wallet: Refactor database cursor into its own object with proper return codes by achow101
  21. DrahtBot cross-referenced this on Dec 16, 2022 from issue Remove Sock::Get() and Sock::Sock() by vasild
  22. DrahtBot cross-referenced this on Dec 16, 2022 from issue Add DataStream without ser-type and ser-version and use it where possible by maflcko
  23. DrahtBot cross-referenced this on Dec 16, 2022 from issue net: Use serialization parameters for CAddress serialization by maflcko
  24. DrahtBot cross-referenced this on Dec 16, 2022 from issue wallet: Load database records in a particular order by achow101
  25. maflcko referenced this in commit 5055d07edf on Dec 16, 2022
  26. DrahtBot cross-referenced this on Dec 16, 2022 from issue Make all networking code mockable by vasild
  27. DrahtBot cross-referenced this on Dec 16, 2022 from issue Try to use posix_fadvise with CBufferedFile by luke-jr
  28. DrahtBot added the label Needs rebase on Dec 16, 2022
  29. hebasto force-pushed on Dec 16, 2022
  30. hebasto cross-referenced this on Dec 16, 2022 from issue refactor: Fix `performance-for-range-copy` in headers by hebasto
  31. hebasto cross-referenced this on Dec 16, 2022 from issue clang-tidy: Force checks for headers in `src/qt` by hebasto
  32. DrahtBot removed the label Needs rebase on Dec 16, 2022
  33. sidhujag referenced this in commit 5bcd427ef5 on Dec 16, 2022
  34. DrahtBot cross-referenced this on Dec 17, 2022 from issue Introduce `MockableDatabase` for wallet unit tests by achow101
  35. maflcko referenced this in commit 6c01323d9d on Dec 17, 2022
  36. DrahtBot added the label Needs rebase on Dec 17, 2022
  37. hebasto force-pushed on Dec 17, 2022
  38. 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 :)

  39. maflcko referenced this in commit cb32328d1b on Dec 17, 2022
  40. hebasto force-pushed on Dec 17, 2022
  41. DrahtBot removed the label Needs rebase on Dec 17, 2022
  42. sidhujag referenced this in commit fd665e9f18 on Dec 17, 2022
  43. sidhujag referenced this in commit 9f664d719e on Dec 17, 2022
  44. DrahtBot cross-referenced this on Dec 29, 2022 from issue ci: Use clang-15 and IWYU v0.19 in "tidy" task by hebasto
  45. DrahtBot cross-referenced this on Jan 4, 2023 from issue test: add end-to-end tests for CConnman and PeerManager by vasild
  46. DrahtBot cross-referenced this on Jan 5, 2023 from issue refactor: add kernel/cs_main.h by fanquake
  47. DrahtBot cross-referenced this on Jan 13, 2023 from issue wallet: Use defined purposes instead of inlining by aureleoules
  48. DrahtBot added the label Needs rebase on Jan 16, 2023
  49. hebasto referenced this in commit b7f6a89a3e on Jan 17, 2023
  50. hebasto force-pushed on Jan 17, 2023
  51. 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.

  52. DrahtBot removed the label Needs rebase on Jan 17, 2023
  53. hebasto force-pushed on Jan 17, 2023
  54. DrahtBot cross-referenced this on Jan 21, 2023 from issue Add fee_est tool for debugging fee estimation code by ryanofsky
  55. DrahtBot added the label Needs rebase on Jan 23, 2023
  56. maflcko referenced this in commit 30f553d457 on Jan 24, 2023
  57. sidhujag referenced this in commit db74fb68cc on Jan 24, 2023
  58. hebasto force-pushed on Jan 24, 2023
  59. hebasto marked this as ready for review on Jan 24, 2023
  60. hebasto commented at 8:59 PM on January 24, 2023: member

    Rebased and ready for reviewing.

  61. DrahtBot removed the label Needs rebase on Jan 24, 2023
  62. 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.

  63. DrahtBot cross-referenced this on Jan 25, 2023 from issue refactor: Remove stray cs_main redundant declaration by maflcko
  64. DrahtBot added the label Needs rebase on Jan 26, 2023
  65. hebasto force-pushed on Jan 26, 2023
  66. 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)

  67. 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.

  68. DrahtBot removed the label Needs rebase on Jan 26, 2023
  69. DrahtBot added the label Needs rebase on Jan 30, 2023
  70. 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
  71. clang-tidy: Force to check all headers b0e916913c
  72. hebasto force-pushed on Jan 31, 2023
  73. 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
  74. hebasto commented at 11:54 AM on January 31, 2023: member

    Rebased. The PR description has been updated.

  75. DrahtBot removed the label Needs rebase on Jan 31, 2023
  76. maflcko approved
  77. 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>

  78. 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

  79. maflcko merged this on Feb 1, 2023
  80. maflcko closed this on Feb 1, 2023
  81. hebasto deleted the branch on Feb 1, 2023
  82. sidhujag referenced this in commit 28e7ecec8d on Feb 1, 2023
  83. hebasto cross-referenced this on Feb 1, 2023 from issue clang-tidy: Add more `performance-*` checks and related fixes by hebasto
  84. hebasto cross-referenced this on Mar 21, 2023 from issue refactor: Use move semantics instead of custom swap functions by hebasto
  85. bitcoin locked this on Mar 5, 2024
Contributors
hebastoDrahtBotmaflckoaureleoulesRandyMcMillanw0xlt
Linked (view graph)
#26642 clang-tidy: Add more `performance-*` checks and related fixes#26703 ci: clang-tidy does not check header files?#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#26749 refactor: Use move semantics instead of custom swap functions#26965 refactor: Remove stray cs_main redundant declaration
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-20 06:53 UTC