Add clang-tidy support for static code analysis
#2648
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikee47
force-pushed
the
feature/clang-tidy
branch
from
d4d6732 to
b16ddf5
Compare
mikee47
force-pushed
the
feature/clang-tidy
branch
from
b16ddf5 to
2bd33be
Compare
mikee47
changed the title
clang-tidy support for static code analysis
Merged
mikee47
force-pushed
the
feature/clang-tidy
branch
2 times, most recently
from
e0b6772 to
4b9fc84
Compare
slaff
pushed a commit
that referenced
this pull request
Apr 22, 2024
This PR proposes some changes to the Hosted classes **Update simpleRPC to master** With clang-tidy #2648 some failures occur in the `simpleRPC` library. Updating this to current master solves the issues. I've also put all the code into the `simpleRPC` namespace which sorts out the conflict with `Vector` but makes it available for use if required. **Tidy HostTests module** Also decode the anonymous 'packet' blob so we can see what's in it and compare with spec. should we wish to update it. **Use abstract base class for callbacks** Simplifies implementation since usually all callbacks are required. Compiler ensures all methods have implementations. `clang-tidy` gave potential memory leak indication because of Delegates. Not sure if that's real but this fixes it.
added 5 commits
April 22, 2024 15:15
Code doesn't get compiled so skip linking stage. Remove `ENABLE_CLANG_TIDY`, use `CLANG_TIDY`, allows specific versions of clang-tidy to be used.
e.g. cannot use `static_cast` so FlashString needs some fudging to get it to compile. Also clang doesn't support constexpr math (yet) so workaround that.
Compiler can optimise ones in headers, but a few others worth doing
Because CSocket virtual destructor calls virtual `close` method which doesn't behave as expected. This one's simple to fix but there are some other cases in the framework (Networking) which will need to be looked at separately.
mikee47
force-pushed
the
feature/clang-tidy
branch
from
4b9fc84 to
1ed7de3
Compare
|
I'm starting to get a feel for the power of this tool! For example: Will apply a single class of fix across the entire codebase. And the results are consistent and work very well. Same is not true for all checks but I think this is probably the way to go when we decide to apply these sorts of changes. What I'm currently not seeing is an easy way to track changes made into submodules... We should consider globally applying these fixes across the codebase (either in this PR or a separate one): |
mikee47
force-pushed
the
feature/clang-tidy
branch
from
1ed7de3 to
96aeb44
Compare
mikee47
changed the title
clang-tidy support for static code analysisclang-tidy support for static code analysis
mikee47
force-pushed
the
feature/clang-tidy
branch
from
96aeb44 to
ca72592
Compare
slaff
approved these changes
Apr 23, 2024
slaff
pushed a commit
that referenced
this pull request
Jun 12, 2024
This PR adds the `CLANG_BUILD` for host builds. The toolchain detection logic is in the main build.mk file as there might be future support for clang toolchains for actual devices. It's also a revision to the existing logic which checks GCC compiler version. Try it out using `make SMING_SOC=host CLANG_BUILD=1`. To build with a specific (installed) version of clang, for example clang-15, use `CLANG_BUILD=15` . Further customisation can be made by editing `Sming/Arch/Host/build.mk`. Clang-tidy support (#2648) is also improved as there are some compiler flag differences between GCC and clang which are now shared between CLANG_TIDY and CLANG_BUILD operation. An extra CI build has been added using clang. Further to #2773, the default toolchain for macos is a version of clang (Apple Clang). This PR doesn't quite support that because there are other issues to address, but it's a step in the right direction.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR allows static code analysis using clang-tidy, suggested by #2616.
Clang has more limited
constexprsupport than GCC so cannot parse some of the FlashString and NanoTime code without modification. However, these changes are only made when__clang__is defined and do not affect regular builds with GCC.This PR also includes some basic code fixes which clang identifies. There are a lot more to look at.
To try it out, build a project with:
Add additional parameters like this:
Notes:
-joption as clang-tidy output and fixes don't get serialised correctly..clang-tidyfile. A custom version can be used and passed on the command line.