-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clang-tidy: apply modernize-loop-convert over a set of files #23443
Conversation
``` ./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-chip-tool-clang/compile_commands.json --export-fixes out/fixes.xml fix ```
``` ./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-all-clusters-clang/compile_commands.json --export-fixes out/fixes.xml fix ```
``` ./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-tests-clang/compile_commands.json --export-fixes out/fixes.xml fix ```
PR #23443: Size comparison from 5030df4 to 8b3d1e3 Increases (4 builds for bl702, linux)
Decreases (14 builds for bl602, bl702, k32w, qpg, telink)
Full report (17 builds for bl602, bl702, k32w, linux, mbed, qpg, telink)
|
PR #23443: Size comparison from 5030df4 to ef77554 Increases (2 builds for bl702)
Decreases (16 builds for bl602, bl702, k32w, nrfconnect, qpg, telink)
Full report (17 builds for bl602, bl702, k32w, mbed, nrfconnect, qpg, telink)
|
PR #23443: Size comparison from 5030df4 to 5b736b5 Increases (14 builds for bl702, cc13x2_26x2, linux, psoc6)
Decreases (37 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, nrfconnect, psoc6, qpg, telink)
Full report (40 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only got partway through, but there are a lot of issues here in terms of the naming and type selection for the loops....
src/credentials/attestation_verifier/DefaultDeviceAttestationVerifier.cpp
Outdated
Show resolved
Hide resolved
@bzbarsky-apple went manually through the changes and used more sane names. Also converted a setter to take a const array to make a const-correctness test better. Overall same change, but with saner names now I hope. Did not try hard for converting auto to const auto &, however some where converted. I am going for "this is slightly better than before". |
PR #23443: Size comparison from 90453bf to 91ce0ae Increases (14 builds for bl702, cc13x2_26x2, linux, psoc6)
Decreases (37 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, nrfconnect, psoc6, qpg, telink)
Full report (40 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
Do we want to update |
I was unsure if forcing to use loop is something that we want everywhere. My instinct was that this does not always feel natural, but at the same time I 100% accepted all automated changes. Would welcome opinions from reviewers here if we should change clang-tidy to enforce or not. Change in BleLayer.cpp was questionable: had to make a static_assert to make the intent clear: seemingly unrelated constants meant |
PR #23443: Size comparison from 706e9bb to 669d4f8 Increases (4 builds for bl702, linux)
Decreases (9 builds for bl702, k32w, nrfconnect, qpg)
Full report (12 builds for bl702, k32w, linux, mbed, nrfconnect, qpg)
|
…-chip#23443) * Modernize using: ``` ./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-chip-tool-clang/compile_commands.json --export-fixes out/fixes.xml fix ``` * Fix using: ``` ./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-all-clusters-clang/compile_commands.json --export-fixes out/fixes.xml fix ``` * Modernize using: ``` ./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-tests-clang/compile_commands.json --export-fixes out/fixes.xml fix ``` * Restyle and remove unused variable * Remove more unused variables for array sizes * Restyle * Start some manual renames of iteration values ... automated tool is quite silly * More manual updates for naming of things. Code should be cleaner now * Restyle * Enforce lowercase naming for cdSigningKey iterator variable * One more pass for cleanups Co-authored-by: Andrei Litvin <andreilitvin@google.com>
…-chip#23443) * Modernize using: ``` ./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-chip-tool-clang/compile_commands.json --export-fixes out/fixes.xml fix ``` * Fix using: ``` ./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-all-clusters-clang/compile_commands.json --export-fixes out/fixes.xml fix ``` * Modernize using: ``` ./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-tests-clang/compile_commands.json --export-fixes out/fixes.xml fix ``` * Restyle and remove unused variable * Remove more unused variables for array sizes * Restyle * Start some manual renames of iteration values ... automated tool is quite silly * More manual updates for naming of things. Code should be cleaner now * Restyle * Enforce lowercase naming for cdSigningKey iterator variable * One more pass for cleanups Co-authored-by: Andrei Litvin <andreilitvin@google.com>
Applied a
modernize-loop-convert
using chip-tool, all-clusters and tests as the sources of files to compile.