Conversation
There was a problem hiding this comment.
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)$' |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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: "*" |
There was a problem hiding this comment.
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: ""
Summary
.clang-tidybased on the configuration used inhw-native-sys/pyptoinclude/andlib/layoutTesting
clang-tidyis not installed in the local environment)