scripted-diff: Use clang-tidy syntax for C++ named arguments (tests only) #23546

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

To allow them being checked by clang-tidy, use a format it can understand.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #23604 (Use Sock in CNode by vasild)
• #23575 (fuzz: Rework FillNode by MarcoFalke)
• #23507 (Refactor: Improve API design of ScriptToUniv, TxToUniv etc to return the UniValue instead of mutating a parameter/reference by mjdietzx)
• #23465 (Remove CTxMemPool params from ATMP by lsilva01)
• #23437 (refactor: AcceptToMemoryPool by lsilva01)
• #23373 (Parse command line arguments from unit and fuzz tests, make addrman consistency check ratio easier to change by vasild)
• #23280 (init: Coalesce Chainstate loading sequence between {,non-}unittest codepaths by dongcarl)
• #22954 ([TESTS] Allow tx_invalid.json tests to include flag rules for if_unset: [A,B,C] then_unset: [D] by JeremyRubin)
• #22924 (refactor: cleanup/followup of Remove -deprecatedrpc=addresses flag by mjdietzx)
• #22876 ([TESTS] Update Transaction Tests to permit setting a flag as always on and disabling the exhaustive failure test by JeremyRubin)
• #22778 (net processing: Reduce resource usage for inbound block-relay-only connections by jnewbery)
• #22702 (Add allocator for node based containers by martinus)
• #21878 (Make all networking code mockable by vasild)
• #21160 (net/net processing: Move tx inventory into net_processing by jnewbery)

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.

Concept ACK, thanks.

Setup clang-tidy:

apt install clang-tidy bear clang
./autogen.sh && ./configure --with-incompatible-bdb CC=clang CXX=clang++
make clean && bear make -j $(nproc)     # For bear 2.x
make clean && bear -- make -j $(nproc)  # For bear 3.x

Full test:

( cd ./src/ && run-clang-tidy  -j $(nproc) )

Diff test:

git diff | ( cd ./src/ && clang-tidy-diff -p2 -j $(nproc) )

ACK fa00447442f22a24e5ca5fc538d0bf7bef575544

I independently recreated the changes done in this PR by:

1. Reseting the commit done in this PR and then,
2. Using the following command.

perl -0777 -pi -e 's:((\(|\{|,)(\n| )*)\/\* ?([^=* ]+) ?\*\/ ?:\1/*\4=*/:g' $( git ls-files ./src/test ./src/wallet/test ) src/test/crypto_tests.cpp src/test/util/setup_common.h

After that I compared the original PR and my recreation and I got the following diff:

--- a/src/test/crypto_tests.cpp
+++ b/src/test/crypto_tests.cpp
@@ -574,10 +574,10 @@ BOOST_AUTO_TEST_CASE(hkdf_hmac_sha256_l32_tests)
 {
     // Use rfc5869 test vectors but truncated to 32 bytes (our implementation only support length 32)
     TestHKDF_SHA256_32(
-                /*IKM=*/"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b",
-                /*salt=*/"000102030405060708090a0b0c",
-                /*info=*/"f0f1f2f3f4f5f6f7f8f9",
-                /* expected OKM */ "3cb25f25faacd57a90434f64d0362f2a2d2d0a90cf1a5a4c5db02d56ecc4c5bf");
+                /*ikm_hex=*/"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b",
+                /*salt_hex=*/"000102030405060708090a0b0c",
+                /*info_hex=*/"f0f1f2f3f4f5f6f7f8f9",
+                /*okm_check_hex=*/"3cb25f25faacd57a90434f64d0362f2a2d2d0a90cf1a5a4c5db02d56ecc4c5bf");
     TestHKDF_SHA256_32(
                 "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f404142434445464748494a4b4c4d4e4f",
                 "606162636465666768696a6b6c6d6e6f707172737475767778797a7b7c7d7e7f808182838485868788898a8b8c8d8e8f909192939495969798999a9b9c9d9e9fa0a1a2a3a4a5a6a7a8a9aaabacadaeaf",
diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
index b583ddb07..4f2ccb6eb 100644
--- a/src/test/util/setup_common.h
+++ b/src/test/util/setup_common.h
@@ -56,7 +56,7 @@ void Seed(FastRandomContext& ctx);
 static inline void SeedInsecureRand(SeedRand seed = SeedRand::SEED)
 {
     if (seed == SeedRand::ZEROS) {
-        g_insecure_rand_ctx = FastRandomContext(/*deterministic=*/true);
+        g_insecure_rand_ctx = FastRandomContext(/*fDeterministic=*/true);
     } else {
         Seed(g_insecure_rand_ctx);
     }

The diff indicates only the explicit name changes and not any functional difference. The new names are the names of the variable in the function definition.

I agree with these changes. Thanks for catching and correcting this, @MarcoFalke!

<p>

ACK https://github.com/bitcoin/bitcoin/pull/23546/commits/fa00447442f22a24e5ca5fc538d0bf7bef575544

Manually tested the scripted diff to verify the produce the same changes.

Though I am getting few errors in the full test like this

$ ( cd ./src/ && run-clang-tidy  -j $(nproc) )
3 warnings and 2 errors generated.
Error while processing /home/raj/github-repo/bitcoin/src/test/bswap_tests.cpp.
Suppressed 3 warnings (3 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
Found compiler error(s).
clang-tidy-12 --use-color -p=/home/raj/github-repo/bitcoin /home/raj/github-repo/bitcoin/src/validation.cpp
error: unknown argument: '-fstack-reuse=none' [clang-diagnostic-error]
error: unsupported option '-fno-extended-identifiers' [clang-diagnostic-error]

And also quite a lot of warnings.

Am I doing clang-tidy wrong?

error: unknown argument: '-fstack-reuse=none' [clang-diagnostic-error]

Did you run configure with clang and cleared the compile_commands.json?

rm compile_commands.json
./autogen.sh && ./configure CC=clang CXX=clang++

ACK fa00447442f22a24e5ca5fc538d0bf7bef575544

ACK fa00447442f22a24e5ca5fc538d0bf7bef575544