Skip to content
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

ci: enable bugprone-* warnings in clang-tidy #5543

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

veqcc
Copy link
Contributor

@veqcc veqcc commented Dec 9, 2024

Description

This PR enables bugprone prefix warnings in clang-tidy-ci.

By specifying bugprone-*, first, all the bugprone prefix warnings are enabled.
After that, the warnings which the current Autoware.universe has are disabled by using - prefix.

The enabled checks are the following.
The current Autoware.universe is free of these warnings.

    bugprone-argument-comment
    bugprone-assert-side-effect
    bugprone-bad-signal-to-kill-thread
    bugprone-bool-pointer-implicit-conversion
    bugprone-copy-constructor-init
    bugprone-dangling-handle
    bugprone-dynamic-static-initializers
    bugprone-fold-init-type
    bugprone-forward-declaration-namespace
    bugprone-forwarding-reference-overload
    bugprone-inaccurate-erase
    bugprone-incorrect-roundings
    bugprone-lambda-function-name
    bugprone-macro-repeated-side-effects
    bugprone-misplaced-operator-in-strlen-in-alloc
    bugprone-misplaced-pointer-arithmetic-in-alloc
    bugprone-misplaced-widening-cast
    bugprone-move-forwarding-reference
    bugprone-multiple-statement-macro
    bugprone-no-escape
    bugprone-not-null-terminated-result
    bugprone-posix-return
    bugprone-redundant-branch-condition
    bugprone-signal-handler
    bugprone-sizeof-container
    bugprone-sizeof-expression
    bugprone-spuriously-wake-up-functions
    bugprone-string-constructor
    bugprone-string-integer-assignment
    bugprone-string-literal-with-embedded-nul
    bugprone-stringview-nullptr
    bugprone-suspicious-enum-usage
    bugprone-suspicious-include
    bugprone-suspicious-memory-comparison
    bugprone-suspicious-memset-usage
    bugprone-suspicious-missing-comma
    bugprone-suspicious-semicolon
    bugprone-suspicious-string-compare
    bugprone-swapped-arguments
    bugprone-terminating-continue
    bugprone-throw-keyword-missing
    bugprone-too-small-loop-variable
    bugprone-undefined-memory-manipulation
    bugprone-undelegated-constructor
    bugprone-unhandled-exception-at-new
    bugprone-unhandled-self-assignment
    bugprone-unused-raii
    bugprone-unused-return-value
    bugprone-use-after-move
    bugprone-virtual-near-miss

How was this PR tested?

I locally tested that this configuration works fine and the current Autoware.universe has no errors/warnings with it.

Notes for reviewers

The number of warnings for each check are:

-bugprone-branch-clone, 31
-bugprone-easily-swappable-parameters, too many
-bugprone-exception-escape, 13
-bugprone-implicit-widening-of-multiplication-result, too many
-bugprone-integer-division, 16
-bugprone-macro-parentheses, 1
-bugprone-narrowing-conversions, too many
-bugprone-parent-virtual-call, 1
-bugprone-reserved-identifier, 10
-bugprone-signed-char-misuse" 2

And bugprone-infinite-loop is suppressed because it takes too long time to run checkings.

Effects on system behavior

Copy link

github-actions bot commented Dec 9, 2024

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@veqcc veqcc self-assigned this Dec 9, 2024
@veqcc veqcc marked this pull request as ready for review December 9, 2024 23:22
@veqcc veqcc requested a review from mitsudome-r as a code owner December 9, 2024 23:22
@veqcc veqcc requested a review from xmfcx December 9, 2024 23:23
@xmfcx
Copy link
Contributor

xmfcx commented Dec 10, 2024

Could you provide the exact steps for me to replicate and test this PR on my local machine?

(you can skip the git checkout parts)

@veqcc
Copy link
Contributor Author

veqcc commented Dec 10, 2024

@xmfcx

Could you provide the exact steps for me to replicate and test this PR on my local machine?

Sure!!
With the following steps, you can check Autoware has no error for the enabled checks.

  1. copy .clang-tidy-ci to .clang-tidy
Checks: "
  -*,
  bugprone-*,
  -bugprone-branch-clone,
  -bugprone-easily-swappable-parameters,
  -bugprone-exception-escape,
  -bugprone-implicit-widening-of-multiplication-result,
  -bugprone-integer-division,
  -bugprone-macro-parentheses,
  -bugprone-narrowing-conversions,
  -bugprone-parent-virtual-call,
  -bugprone-reserved-identifier,
  -bugprone-signed-char-misuse"

WarningsAsErrors: "*"

ExtraArgs:
  - -std=c++17
  - -Wno-c11-extensions
  1. build autoware with compile_commands.json
$ colcon build --symlink-install --cmake-args -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -DCMAKE_BUILD_TYPE=Release
  1. set environment variable
$ export FILES=($(find src/universe/autoware.universe/ -name '*.cpp' -not -path '*/test/*' -not -path '*/examples/*'))
  1. run clang-tidy ( .clang-tidy will automatically loaded )
$ run-clang-tidy -j $(nproc) -p build/ "${FILES[@]}"

As I have written in Notes, this command takes much long time (some hours in my env)...

@xmfcx
Copy link
Contributor

xmfcx commented Dec 10, 2024

@veqcc -san I will test this today, sorry for being late.

@xmfcx
Copy link
Contributor

xmfcx commented Dec 10, 2024

Hello @veqcc -san, thank you for providing the steps.

Only difference is I build with colcon build --symlink-install --event-handlers=console_cohesion+ --cmake-args -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_EXPORT_COMPILE_COMMANDS=1

I followed them but got all these errors: https://gist.github.com/xmfcx/9c51facc1147f0ea9734bd1b91a02db2

Am I missing something?

I've also checked https://github.com/autowarefoundation/autoware-github-actions/blob/main/clang-tidy/action.yaml and couldn't figure out what might me missing.

I am on Ubuntu 22.04 and installed sudo apt install clang-tidy libomp-dev.

My .bashrc has:

# Ccache
export CC="/usr/lib/ccache/gcc"
export CXX="/usr/lib/ccache/g++"
export CCACHE_DIR="$HOME/.cache/ccache/"

I wonder if this might have caused it 🤔

@veqcc
Copy link
Contributor Author

veqcc commented Dec 11, 2024

@xmfcx

Somehow, clang-tidy seems not recognizing C++ headers 🥹
Could you try adding extra args?

  - -I/usr/include/c++/11
  - -I/usr/include/x86_64-linux-gnu/c++/11

So the .clang-tidy file looks like

Checks: "
  -*,
  bugprone-*,
  -bugprone-branch-clone,
  -bugprone-easily-swappable-parameters,
  -bugprone-exception-escape,
  -bugprone-implicit-widening-of-multiplication-result,
  -bugprone-integer-division,
  -bugprone-macro-parentheses,
  -bugprone-narrowing-conversions,
  -bugprone-parent-virtual-call,
  -bugprone-reserved-identifier,
  -bugprone-signed-char-misuse"

WarningsAsErrors: "*"

ExtraArgs:
  - -std=c++17
  - -Wno-c11-extensions
  - -I/usr/include/c++/11
  - -I/usr/include/x86_64-linux-gnu/c++/11

@veqcc
Copy link
Contributor Author

veqcc commented Dec 11, 2024

@xmfcx
I additionally suppressed bugprone-infinite-loop check, because I found it takes so much time to run.
Without it, run-clang-tidy for all packages in Autoware.universe completes within 30 minutes in my local env.

@xmfcx
Copy link
Contributor

xmfcx commented Dec 11, 2024

In the latest main of current time, this has passed with the following mods:

My .clang-tidy file:

Checks: "
  -*,
  bugprone-*,
  -bugprone-branch-clone,
  -bugprone-easily-swappable-parameters,
  -bugprone-exception-escape,
  -bugprone-implicit-widening-of-multiplication-result,
  -bugprone-infinite-loop,
  -bugprone-integer-division,
  -bugprone-macro-parentheses,
  -bugprone-narrowing-conversions,
  -bugprone-parent-virtual-call,
  -bugprone-reserved-identifier,
  -bugprone-signed-char-misuse"

WarningsAsErrors: "*"

ExtraArgs:
  - -std=c++17
  - -Wno-c11-extensions
  - -I/usr/include/c++/11
  - -I/usr/include/x86_64-linux-gnu/c++/11
 # I have these in my .bashrc
export CC="/usr/lib/ccache/gcc"
export CXX="/usr/lib/ccache/g++"
export CCACHE_DIR="$HOME/.cache/ccache/"
export CMAKE_EXPORT_COMPILE_COMMANDS=1

# Install stuff
sudo apt update
sudo apt install clang-tidy libomp-dev

# Commands
colcon build --symlink-install --event-handlers=console_cohesion+ --cmake-args -DCMAKE_BUILD_TYPE=RelWithDebInfo

export FILES=($(find src/universe/autoware.universe/ -name '*.cpp' -not -path '*/test/*' -not -path '*/examples/*'))

run-clang-tidy -j $(nproc) -p build/ "${FILES[@]}"

To measure the time it took, I ran it with:

time run-clang-tidy -j $(nproc) -p build/ "${FILES[@]}"

And it took about 11mins. (I was using a 5900x 12c24t cpu)

real	10m53,005s
user	238m6,803s
sys  	10m53,759s

Results(no errors): https://gist.github.com/xmfcx/0733a689acdf7cfb4479a4e118b5bfdd 🎊

@xmfcx xmfcx merged commit f4698fd into autowarefoundation:main Dec 11, 2024
20 checks passed
@veqcc veqcc deleted the ci/clang-tidy branch December 11, 2024 11:03
youtalk added a commit to youtalk/autoware that referenced this pull request Dec 19, 2024
* feat(.github): push version tag images (autowarefoundation#5522)

* add tag pattern

Signed-off-by: Yutaka Kondo <[email protected]>

* fix syntax

Signed-off-by: Yutaka Kondo <[email protected]>

* add tags

Signed-off-by: Yutaka Kondo <[email protected]>

* fix prefix and suffix

Signed-off-by: Yutaka Kondo <[email protected]>

---------

Signed-off-by: Yutaka Kondo <[email protected]>

* feat(.github): add `on:push:branches:` condition to `autoware-base` workflow (autowarefoundation#5524)

add paths arg

Signed-off-by: Yutaka Kondo <[email protected]>

* fix(.github): not push manifest if eithor `amd64` or `arm64` image is missing (autowarefoundation#5525)

not push manifest if eithor amd64 or arm64 is missing

Signed-off-by: Yutaka Kondo <[email protected]>

* chore(autoware.repos): fix autoware.core commit hash (autowarefoundation#5530)

Signed-off-by: Ryohsuke Mitsudome <[email protected]>

* feat(.git): delete `-amd64` and  `-arm64` image tags after pushing manifest (autowarefoundation#5526)

* not push manifest if eithor amd64 or arm64 is missing

Signed-off-by: Yutaka Kondo <[email protected]>

* delete -amd64 and -arm64 tags

Signed-off-by: Yutaka Kondo <[email protected]>

---------

Signed-off-by: Yutaka Kondo <[email protected]>

* chore(.github): skip `free-disk-space` action if there are no changed files (autowarefoundation#5536)

skip free-disk-space action

Signed-off-by: Yutaka Kondo <[email protected]>

* ci: enable bugprone-* warnings in clang-tidy (autowarefoundation#5543)

Signed-off-by: Ryuta Kambe <[email protected]>

* feat(autoware.repos): use version tag for sensor_kits (autowarefoundation#5554)

Signed-off-by: Ryohsuke Mitsudome <[email protected]>

* ci(pre-commit-ansible): autoupdate (autowarefoundation#5552)

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions <[email protected]>

* chore(deps): bump peter-evans/create-pull-request from 6 to 7 (autowarefoundation#5162)

Bumps [peter-evans/create-pull-request](https://github.com/peter-evans/create-pull-request) from 6 to 7.
- [Release notes](https://github.com/peter-evans/create-pull-request/releases)
- [Commits](peter-evans/create-pull-request@v6...v7)

---
updated-dependencies:
- dependency-name: peter-evans/create-pull-request
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ci(pre-commit-optional): autoupdate (autowarefoundation#5401)

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions <[email protected]>

* chore: sync files (autowarefoundation#5541)

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions <[email protected]>

* chore: sync files (autowarefoundation#5572)

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions <[email protected]>

* fix(.github): fix CODEOWNERS (autowarefoundation#5575)

Update CODEOWNERS

* fix: update branch of scenario_simulator_v2 to master (autowarefoundation#5570)

Signed-off-by: Fumiya Watanabe <[email protected]>

* feat(autoware.repos): remove `autoware_common` from `autoware.repos` (autowarefoundation#5578)

* chore(.devcontainer): add `core-devel` development container (autowarefoundation#5574)

add core-devel devcontainer

Signed-off-by: Yutaka Kondo <[email protected]>

---------

Signed-off-by: Yutaka Kondo <[email protected]>
Signed-off-by: Ryohsuke Mitsudome <[email protected]>
Signed-off-by: Ryuta Kambe <[email protected]>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Fumiya Watanabe <[email protected]>
Co-authored-by: Ryohsuke Mitsudome <[email protected]>
Co-authored-by: Ryuta Kambe <[email protected]>
Co-authored-by: awf-autoware-bot[bot] <94889083+awf-autoware-bot[bot]@users.noreply.github.com>
Co-authored-by: github-actions <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Fumiya Watanabe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants