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

romanz commented at 4:11 AM on September 11, 2024: contributor

No description provided.

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.

DrahtBot added the label RPC/REST/ZMQ on Sep 11, 2024

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.

DrahtBot added the label CI failed on Sep 11, 2024

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

romanz force-pushed on Sep 12, 2024

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.

romanz force-pushed on Sep 12, 2024

romanz renamed this:
~~http: Use 'starts_with' for matching URI prefix~~
tidy: Use 'starts_with' instead of substr/find
on Sep 12, 2024

maflcko removed the label RPC/REST/ZMQ on Sep 12, 2024

DrahtBot removed the label CI failed on Sep 13, 2024

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

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.

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

romanz force-pushed on Sep 14, 2024

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

stickies-v approved

stickies-v commented at 11:06 AM on September 16, 2024: contributor

ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3

DrahtBot requested review from jonatack on Sep 16, 2024

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

DrahtBot added the label Refactoring on Sep 16, 2024

hebasto approved

hebasto commented at 11:17 AM on September 16, 2024: member

ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3, I have reviewed the code and it looks OK.

jonatack commented at 2:04 PM on September 16, 2024: member

re-ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3 only change since my previous ACK is the commit message

achow101 commented at 7:35 PM on September 16, 2024: member

ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3

achow101 merged this on Sep 16, 2024

achow101 closed this on Sep 16, 2024

romanz deleted the branch on Sep 16, 2024

romanz commented at 7:51 PM on September 16, 2024: contributor

Many thanks!

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:

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

TheCharlatan referenced this in commit 8bb47d4c2c on Nov 2, 2024

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:

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

Addressed in #31306.

bug-castercv502 referenced this in commit 403bebd591 on Sep 28, 2025

bitcoin locked this on Nov 17, 2025