Clang tidy fixes step 1 #32
Conversation
This warning occurs:
~~~
test $? -eq 0 || no_result
^-- SC2320 (warning): This $? refers to echo/printf, not a previous command. Assign to variable to avoid it being overwritten.
~~~
See https://www.shellcheck.net/wiki/SC2320 for details.
But in this case the return code of echo is really checked:
to make sure that the file redirection worked.
To avoid the, otherwise useful, ShellCheck warning,
simply pipe through `cat` for these cases.
MegaLinter v6.14.0 does also check file name spelling.
jtxa
force-pushed
the
clang-tidy-fixes-step1
branch
from
8f8b95a
to
d157112
Compare
GitHub advises to do a `apt-get update` first. https://docs.github.com/en/actions/using-github-hosted-runners/customizing-github-hosted-runners
|
Created a new PR #33 for the CI fixes. That one should be merged first. |
|
On MacOS these changes causes errors and warnings like: I need to take a look how this has to be solved. |
Command: ~~~sh run-clang-tidy-14 -header-filter='.*' -checks='-*,modernize-deprecated-headers,readability-duplicate-include' -fix ~~~
Command: ~~~sh run-clang-tidy-14 -header-filter='.*' -checks='-*,modernize-use-auto,readability-qualified-auto' -fix ~~~ Afterwards remove obsolete newlines manually.
Command: ~~~sh run-clang-tidy-14 -header-filter='.*' -checks='-*,readability-non-const-parameter,readability-make-member-function-const' -fix ~~~
Command: ~~~sh run-clang-tidy-14 -header-filter='.*' -checks='-*,modernize-make-shared,modernize-use-emplace,modernize-return-braced-init-list,modernize-use-bool-literals,modernize-raw-string-literal' -fix ~~~
Command: ~~~sh run-clang-tidy-14 -header-filter='.*' -checks='-*,modernize-redundant-void-arg' -fix ~~~
Command: ~~~sh run-clang-tidy-14 -header-filter='.*' -checks='-*,readability-uppercase-literal-suffix' -fix ~~~
Command: ~~~sh run-clang-tidy-14 -header-filter='.*' -checks='-*,readability-redundant-access-specifiers,readability-container-size-empty,readability-static-accessed-through-instance,readability-delete-null-pointer,readability-isolate-declaration' -fix ~~~ Afterwards fix white spaces (newline, indentation).
Command: ~~~sh run-clang-tidy-14 -header-filter='.*' -checks='-*,bugprone-macro-parentheses' -fix ~~~ Manually replace duplicate macros by include.
Fix the following warning: ~~~ return type 'const std::string' (aka 'const basic_string<char>') is 'const'-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type,-warnings-as-errors] ~~~
Fix the following warnings: ~~~ function 'memcmp' is called without explicitly comparing result [bugprone-suspicious-string-compare,-warnings-as-errors] function 'strcmp' is called without explicitly comparing result [bugprone-suspicious-string-compare,-warnings-as-errors] ~~~
Add more checks and disable those not fixed yet.
|
This is huge (but sensational Josef!). It's going to take a while to go through. Nights are mostly available, otherwise, I'll definitely be able to pour through on the upcoming weekend. |
jtxa
force-pushed
the
clang-tidy-fixes-step1
branch
from
d157112
to
a18500b
Compare
|
I rebased it on top of #33 first (no code changes), and added the fix for MacOS: The review needs to be done per commit, that should be easier.
|
Should have explained the current state of the rules. Today I use the default + those rules we currently check. Perhaps it's better to enable all, and then disable those we really don't want. The initial This is what I guessed: Enabled:
Currently not enabled:
Irrelevant:
|
Here are some fixes based on clang-tidy warnings. Mostly auto-fixes, please see commit messages for details.
About half of the messages as discussed in #26 are fixed now:
Additionally small manual fixes for:
Further changes were needed to get the CI running OK again.
There were some updates to MegaLinter.
ATM the ubuntu mirror from MS Azure seems to be not reachable for the GitHub runner. So the tests on Linux may fail. Perhaps a temporary problem, I'll try to run them again later. If necessary I'll add an extra commit, that changes the mirror used.