doc: document clang tidy named args #24665

pull fanquake wants to merge 2 commits into bitcoin:master from fanquake:document_clang_tidy changing 2 files +54 −8

fanquake commented at 8:25 AM on March 25, 2022: member

The documentation, and a single commit extracted from #24661.

Motivation:

Incorrect named args are source of bugs, like #22979.

To allow them being checked by clang-tidy, use a format it can understand.
doc: Document clang-tidy in dev notes 67f654ef61
addrman: fix incorrect named args 7e22d80af3
MarcoFalke approved
MarcoFalke commented at 8:33 AM on March 25, 2022: member

lgtm

in src/addrman.cpp:958 in 7e22d80af3

 961 | -                               /*bucket=*/bucket,
 962 | -                               /*position=*/addr_info->GetBucketPosition(nKey, true, bucket));
 963 | +        return AddressPosition(/*tried_in=*/false,
 964 | +                               /*multiplicity_in=*/addr_info->nRefCount,
 965 | +                               /*bucket_in=*/bucket,
 966 | +                               /*position_in=*/addr_info->GetBucketPosition(nKey, true, bucket));

MarcoFalke commented at 8:36 AM on March 25, 2022:

unrelated comment:

If msvc was less crappy, we could use designated initializers to enforce named args in constructors without clang-tidy, see #24531

DrahtBot added the label P2P on Mar 25, 2022
glozow removed the label P2P on Mar 25, 2022
glozow added the label Docs on Mar 25, 2022
glozow commented at 2:14 PM on March 25, 2022: member

Concept ACK, would be nice to be able to point to developer notes when someone asks why I'm formatting arg names this way. The instructions look correct. Were you planning on fixing all of the clang-tidy violations or just these in addrman.cpp?
fanquake commented at 2:50 PM on March 25, 2022: member

Were you planning on fixing all of the clang-tidy violations or just these in addrman.cpp?

and a single commit extracted from #24661.

The rest of the changes are in #24661. That will likely need rebasing a number of times, so I just want to get the docs, and a demonstration commit in for now.
glozow commented at 2:58 PM on March 25, 2022: member

The rest of the changes are in #24661. That will likely need rebasing a number of times, so I just want to get the docs, and a demonstration commit in for now.

Ok gotchu makes sense
glozow commented at 3:01 PM on March 25, 2022: member

ACK 7e22d80af333b202939bcb2631082006c097bf22
fanquake merged this on Mar 25, 2022
fanquake closed this on Mar 25, 2022
sidhujag referenced this in commit 24a5e0f92b on Apr 2, 2022
fanquake deleted the branch on Apr 20, 2022
DrahtBot locked this on Apr 20, 2023

Contributors

Labels

Linked (view graph)

#22979 doc: Fix walletdisplayaddress RPC doc #24661 refactor: Use clang-tidy syntax for C++ named arguments