Skip to content

chore: add clang-tidy configuration#442

Open
zhangstevenunity wants to merge 3 commits intomainfrom
codex/add-clang-tidy-config
Open

chore: add clang-tidy configuration#442
zhangstevenunity wants to merge 3 commits intomainfrom
codex/add-clang-tidy-config

Conversation

@zhangstevenunity
Copy link
Copy Markdown
Collaborator

Summary

  • add a repository-level .clang-tidy based on the configuration used in hw-native-sys/pypto
  • adapt the header filter to this repository's include/ and lib/ layout
  • keep the same enabled/disabled check groups so local and CI static analysis can use a shared baseline

Testing

  • not run (clang-tidy is not installed in the local environment)

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new .clang-tidy configuration file to enforce various coding standards and static analysis checks. The review feedback highlights potential issues with the current configuration: the HeaderFilterRegex is overly broad and may include system headers, the misc-include-cleaner check might be too aggressive for the current state of the codebase, and enabling WarningsAsErrors globally could lead to immediate CI failures before existing issues are resolved.

-portability-template-virtual-member-function

WarningsAsErrors: "*"
HeaderFilterRegex: '.*(include[/\\]|lib[/\\]).*\.(h|hpp)$'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The current HeaderFilterRegex is too broad because the leading .* followed by include or lib will match system headers (e.g., /usr/include/stdio.h), causing clang-tidy to report diagnostics for external code. Combined with WarningsAsErrors: "*", this will likely break the build. It is better to use a regex that specifically targets the project's own header directories by including unique path components like PTO or CAPI.

HeaderFilterRegex: '.*(include|lib)[/\\].*(PTO|CAPI|Bindings)[/\\].*\.(h|hpp)$'

modernize-*,
performance-*,
portability-*,
misc-include-cleaner,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The misc-include-cleaner check is very strict and often requires manual annotations (such as // IWYU pragma: keep) to handle false positives where it incorrectly identifies an include as unused or missing. If the codebase has not been prepared for this check, it may generate a large number of errors. Consider disabling it until the initial cleanup is complete.

-performance-inefficient-string-concatenation,
-portability-template-virtual-member-function

WarningsAsErrors: "*"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Enabling WarningsAsErrors: "*" on a new configuration that has not been tested locally (as noted in the PR description) is highly likely to break the CI build. It is recommended to first introduce the configuration with warnings only, fix existing violations, and then enable this option to maintain a clean baseline.

WarningsAsErrors: ""

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.

1 participant