refactor: add clang-tidy modernize-use-starts-ends-with check #30868

pull romanz wants to merge 1 commits into bitcoin:master from romanz:starts-with changing 4 files +4 −3
  1. romanz commented at 4:11 am on September 11, 2024: contributor
  2. DrahtBot commented at 4:11 am on September 11, 2024: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK stickies-v, hebasto, jonatack, achow101

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #30679 (fix: handle invalid -rpcbind port earlier by tdb3)

    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. DrahtBot added the label RPC/REST/ZMQ on Sep 11, 2024
  4. maflcko commented at 5:56 am on September 11, 2024: member
    If there is support for this change, it may be better to use and enable https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-starts-ends-with.html . Otherwise, follow-up pull requests may be created for each instance individually. Also, newly introduced code may re-introduce new instances.
  5. DrahtBot added the label CI failed on Sep 11, 2024
  6. maflcko commented at 9:03 pm on September 11, 2024: member
    Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
  7. romanz force-pushed on Sep 12, 2024
  8. stickies-v commented at 1:06 pm on September 12, 2024: contributor
    Code LGTM eb43872b84bfc4d09fe38ce0cb74722f6f9842da. Commit message and PR title need updating, this is a tidy change now affecting multiple modules, not an http change.
  9. romanz force-pushed on Sep 12, 2024
  10. romanz renamed this:
    http: Use 'starts_with' for matching URI prefix
    tidy: Use 'starts_with' instead of substr/find
    on Sep 12, 2024
  11. maflcko removed the label RPC/REST/ZMQ on Sep 12, 2024
  12. DrahtBot removed the label CI failed on Sep 13, 2024
  13. jonatack commented at 4:05 pm on September 13, 2024: member

    ACK 22bc9fdca314549bf204d3e3f577d7f014858f42

    Maybe name this PR (and the commit, if you need to retouch):

    tidy: add clang-tidy modernize-use-starts-ends-with check

  14. jonatack commented at 4:10 pm on September 13, 2024: member

    The red CI is an unrelated timeout.

    I’m still adapting to the new build system, but fwiw, locally it looks like no unit tests fail if I flip the changed conditionals to the opposite boolean. Functional tests do fail.

  15. tidy: add clang-tidy `modernize-use-starts-ends-with` check fc7b507e9a
  16. romanz force-pushed on Sep 14, 2024
  17. romanz renamed this:
    tidy: Use 'starts_with' instead of substr/find
    tidy: add clang-tidy `modernize-use-starts-ends-with` check
    on Sep 14, 2024
  18. stickies-v approved
  19. stickies-v commented at 11:06 am on September 16, 2024: contributor
    ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3
  20. DrahtBot requested review from jonatack on Sep 16, 2024
  21. DrahtBot renamed this:
    tidy: add clang-tidy `modernize-use-starts-ends-with` check
    refactor: add clang-tidy `modernize-use-starts-ends-with` check
    on Sep 16, 2024
  22. DrahtBot added the label Refactoring on Sep 16, 2024
  23. hebasto approved
  24. hebasto commented at 11:17 am on September 16, 2024: member
    ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3, I have reviewed the code and it looks OK.
  25. jonatack commented at 2:04 pm on September 16, 2024: member
    re-ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3 only change since my previous ACK is the commit message
  26. achow101 commented at 7:35 pm on September 16, 2024: member
    ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3
  27. achow101 merged this on Sep 16, 2024
  28. achow101 closed this on Sep 16, 2024
  29. romanz deleted the branch on Sep 16, 2024
  30. romanz commented at 7:51 pm on September 16, 2024: contributor
    Many thanks!
  31. fanquake commented at 11:16 am on October 1, 2024: member

    Seems like newer Clang 19 has improved this check, and throws out more (historical) instances to change. i.e:

     0bitcoin/src/torcontrol.cpp:359:27: error: use starts_with instead of compare() == 0 [modernize-use-starts-ends-with,-warnings-as-errors]
 1  359 |             if (0 == line.compare(0, 20, "net/listeners/socks=")) {
 2      |                 ~~~~      ^~~~~~~~~~~~~~                       ~
 3      |                           starts_with(                         )
 4bitcoin/src/torcontrol.cpp:370:38: error: use starts_with instead of compare() == 0 [modernize-use-starts-ends-with,-warnings-as-errors]
 5  370 |                     if (0 == portstr.compare(0, 10, "127.0.0.1:")) {
 6      |                         ~~~~         ^~~~~~~~~~~~~~             ~
 7      |                                      starts_with(               )
 8bitcoin/src/core_read.cpp:42:25: error: use starts_with instead of compare() == 0 [modernize-use-starts-ends-with,-warnings-as-errors]
 9   42 |             if (strName.compare(0, 3, "OP_") == 0) { // strName starts with "OP_"
10      |                         ^~~~~~~~~~~~~      ~~~~~~
11      |                         starts_with(       )
12bitcoin/src/wallet/walletdb.cpp:1016:27: error: use starts_with instead of compare() == 0 [modernize-use-starts-ends-with,-warnings-as-errors]
13 1016 |         } else if (strKey.compare(0, 2, "rr") == 0) {
14      |                           ^~~~~~~~~~~~~     ~~~~~~
15      |                           starts_with(      )
16<snip the rest>
  32. TheCharlatan referenced this in commit 8bb47d4c2c on Nov 2, 2024
  33. hebasto commented at 4:26 pm on November 17, 2024: member

    Seems like newer Clang 19 has improved this check, and throws out more (historical) instances to change. i.e:

     0bitcoin/src/torcontrol.cpp:359:27: error: use starts_with instead of compare() == 0 [modernize-use-starts-ends-with,-warnings-as-errors]
 1  359 |             if (0 == line.compare(0, 20, "net/listeners/socks=")) {
 2      |                 ~~~~      ^~~~~~~~~~~~~~                       ~
 3      |                           starts_with(                         )
 4bitcoin/src/torcontrol.cpp:370:38: error: use starts_with instead of compare() == 0 [modernize-use-starts-ends-with,-warnings-as-errors]
 5  370 |                     if (0 == portstr.compare(0, 10, "127.0.0.1:")) {
 6      |                         ~~~~         ^~~~~~~~~~~~~~             ~
 7      |                                      starts_with(               )
 8bitcoin/src/core_read.cpp:42:25: error: use starts_with instead of compare() == 0 [modernize-use-starts-ends-with,-warnings-as-errors]
 9   42 |             if (strName.compare(0, 3, "OP_") == 0) { // strName starts with "OP_"
10      |                         ^~~~~~~~~~~~~      ~~~~~~
11      |                         starts_with(       )
12bitcoin/src/wallet/walletdb.cpp:1016:27: error: use starts_with instead of compare() == 0 [modernize-use-starts-ends-with,-warnings-as-errors]
13 1016 |         } else if (strKey.compare(0, 2, "rr") == 0) {
14      |                           ^~~~~~~~~~~~~     ~~~~~~
15      |                           starts_with(      )
16<snip the rest>

    Addressed in #31306.


romanz DrahtBot maflcko stickies-v jonatack hebasto achow101 fanquake


jonatack

Labels
Refactoring

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: 2024-11-23 18:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me