Enable clang-tidy bugprone-argument-comment and fix violations

[maflcko]

Named arguments can be dangerous when they are wrong, because they are not enforced by the compiler. Currently there are only minor typos, no actual bugs.

Fix the typos and add the .clang-tidy file to make it easier to find them in the future.

[maflcko]

To test on Ubuntu:

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
( cd ./src/ && run-clang-tidy  -j $(nproc) )

[kiminuo]

Is it possibly instead of:

make clean && bear make -j $(nproc)
->
make clean && bear -- make -j $(nproc)

?

(https://github.com/rizsotto/Bear#how-to-use)

edit: I think there are two more instances where the parameter names are not entirely correct:

• https://github.com/MarcoFalke/bitcoin-core/blob/fa91f31752e9387548fa6b932162a12d22c8e211/src/qt/test/addressbooktests.cpp#L115
• https://github.com/MarcoFalke/bitcoin-core/blob/fa91f31752e9387548fa6b932162a12d22c8e211/src/qt/test/wallettests.cpp#L170

[DrahtBot]

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #22019 (wallet: Introduce SelectionResult for encapsulating a coin selection solution by achow101)

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.

[maflcko]

@kiminuo

No, that doesn't work for me:

bear: Internal error.
Traceback (most recent call last):
  File "/usr/local/bin/bear", line 265, in wrapper
    return function(*args, **kwargs)
  File "/usr/local/bin/bear", line 291, in intercept_build
    exit_code, current = capture(args, category)
  File "/usr/local/bin/bear", line 315, in capture
    exit_code = run_build(args.build, env=environment)
  File "/usr/local/bin/bear", line 189, in run_build
    exit_code = subprocess.call(command, *args, **kwargs)
  File "/usr/lib64/python3.9/subprocess.py", line 349, in call
    with Popen(*popenargs, **kwargs) as p:
  File "/usr/lib64/python3.9/subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib64/python3.9/subprocess.py", line 1821, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '--'
bear: Please run this command again and turn on verbose mode (add '-vvvv' as argument).

[kiminuo]

My bear version is bear 3.0.8 and I have Ubuntu 21.04. Anyway, bear make -j $(nproc) does not work for me, only bear -- make -j $(nproc). Maybe there was a breaking change in bear command. I don't know.

edit: Yes, https://github.com/rizsotto/Bear/pull/388/files

[maflcko]

@kiminuo Thanks, updated comment

[maflcko]

edit: I think there are two more instances where the parameter names are not entirely correct:

Thanks, fixed

[practicalswift]

Concept ACK

Could add this specific clang-tidy check as part of the CI lint pass to reduce the risk of introduction of bugprone argument comments going forward?

[maflcko]

Could add this specific clang-tidy check as part of the CI lint pass to reduce the risk of introduction of bugprone argument comments going forward?

Seems like something that should be done in a separate pull request to keep the discussion here focussed on adding the flag and fixing the violations.

[practicalswift]

cr ACK fa57fa1

Could add this specific clang-tidy check as part of the CI lint pass to reduce the risk of introduction of bugprone argument comments going forward?

Seems like something that should be done in a separate pull request to keep the discussion here focussed on adding the flag and fixing the violations.

Makes sense! I'd gladly review such PR FWIW :)

[fanquake]

ACK fa57fa1