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

[bazel] Add aspects to run clang-tidy #18537

Merged
merged 1 commit into from
May 18, 2023

Conversation

dmcardle
Copy link
Contributor

@dmcardle dmcardle commented May 11, 2023

This commit adds "fix" and "check" aspects that run clang-tidy.

Example usage:

./bazelisk.sh build --config=clang_tidy_fix \
  --config=riscv32 //sw/device/lib/base:memory

./bazelisk.sh build --config=clang_tidy_check \
  --config=riscv32 //sw/device/lib/base:memory

This also adds a new target, //quality:clang_tidy_check, which runs
clang-tidy against a very incomplete list of hand-picked targets.

@dmcardle dmcardle force-pushed the dmcardle/clang-tidy branch 3 times, most recently from b9f411b to 395a187 Compare May 11, 2023 19:57
@dmcardle dmcardle mentioned this pull request May 11, 2023
@dmcardle dmcardle force-pushed the dmcardle/clang-tidy branch 2 times, most recently from b4486b3 to 0614f67 Compare May 12, 2023 21:13
@dmcardle dmcardle changed the title [bazel] Add clang-tidy aspect and fix strict prototype warnings [bazel] Add aspects to run clang-tidy May 12, 2023
@dmcardle dmcardle force-pushed the dmcardle/clang-tidy branch from 0614f67 to 4d3aaee Compare May 12, 2023 21:15
@dmcardle dmcardle marked this pull request as ready for review May 12, 2023 21:15
@dmcardle dmcardle requested a review from cfrantz as a code owner May 12, 2023 21:15
@dmcardle dmcardle requested a review from alphan May 12, 2023 21:15
@dmcardle
Copy link
Contributor Author

Hey folks, PTAL when you get a chance! This is half-baked right now, but with Bazel's --keep_going flag, it's enough to make some automated fixes like -Wstrict-prototypes.

@dmcardle dmcardle force-pushed the dmcardle/clang-tidy branch 3 times, most recently from 23853ce to f7bc9c3 Compare May 15, 2023 14:07
This commit adds "fix" and "check" aspects that run clang-tidy.

Example usage:

    ./bazelisk.sh build --config=clang_tidy_fix \
      --config=riscv32 //sw/device/lib/base:memory

    ./bazelisk.sh build --config=clang_tidy_check \
      --config=riscv32 //sw/device/lib/base:memory

This also adds a new target, //quality:clang_tidy_check, which runs
clang-tidy against a very incomplete list of hand-picked targets.

Signed-off-by: Dan McArdle <[email protected]>
@dmcardle dmcardle force-pushed the dmcardle/clang-tidy branch from f7bc9c3 to 9e554af Compare May 15, 2023 18:43
@dmcardle dmcardle requested a review from milesdai May 17, 2023 14:48
Copy link
Contributor

@milesdai milesdai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The aspect setup LGTM. I don't know enough about CcInfo to say more about the collection of the compiler flags and options.

acquire_lock(args.lock_file)

compile_commands = Path("compile_commands.json")
cleanup_func = maybe_rename_path(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no need to address in this PR, but this might be a good use case for a with context manager to handle the cleanup.

@dmcardle dmcardle merged commit 74f6805 into lowRISC:master May 18, 2023
@dmcardle dmcardle deleted the dmcardle/clang-tidy branch May 18, 2023 21:29
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