Add tidy.md document
Test: make Change-Id: Ic2198532be4135401faffdda34d0e108c57b241c
This commit is contained in:
Chih-Hung Hsieh
parent
1785fcb171
commit
22c27e8458
2 changed files with 245 additions and 0 deletions
|
|
@ -1,2 +1,3 @@
|
|||
per-file map_files.md = danalbert@google.com, enh@google.com, jiyong@google.com
|
||||
per-file native_code_coverage.md = pirama@google.com, srhines@google.com, allenhair@google.com
|
||||
per-file tidy.md = chh@google.com, srhines@google.com
|
||||
|
|
|
|||
244
docs/tidy.md
Normal file
244
docs/tidy.md
Normal file
|
|
@ -0,0 +1,244 @@
|
|||
# Clang-Tidy Rules and Checks
|
||||
|
||||
Android C/C++ source files can be checked by clang-tidy for issues like
|
||||
coding style, error-prone/performance patterns, and static flow analysis.
|
||||
See the official
|
||||
[clang-tidy document](https://clang.llvm.org/extra/clang-tidy)
|
||||
and list of
|
||||
[clang-tidy checks](https://clang.llvm.org/extra/clang-tidy/checks/list.html).
|
||||
|
||||
## Global defaults for Android builds
|
||||
|
||||
The simplest way to enable clang-tidy checks is
|
||||
to set environment variable `WITH_TIDY`.
|
||||
```
|
||||
$ WITH_TIDY=1 make
|
||||
```
|
||||
|
||||
This will turn on the global default to run clang-tidy for every required
|
||||
C/C++ source file compilation. The global default clang-tidy checks
|
||||
do not include time-consuming static analyzer checks. To enable those
|
||||
checks, set the `CLANG_ANALYZER_CHECKS` variable.
|
||||
```
|
||||
$ WITH_TIDY=1 CLANG_ANALYZER_CHECKS=1 make
|
||||
```
|
||||
|
||||
The default global clang-tidy checks and flags are defined in
|
||||
[build/soong/cc/config/tidy.go](https://android.googlesource.com/platform/build/soong/+/refs/heads/master/cc/config/tidy.go).
|
||||
|
||||
|
||||
## Module clang-tidy properties
|
||||
|
||||
The global default can be overwritten by module properties in Android.bp.
|
||||
|
||||
### `tidy` and `tidy_checks`
|
||||
|
||||
For example, in
|
||||
[system/bpf/Android.bp](https://android.googlesource.com/platform/system/bpf/+/refs/heads/master/Android.bp),
|
||||
clang-tidy is enabled explicitly and with a different check list:
|
||||
```
|
||||
cc_defaults {
|
||||
name: "bpf_defaults",
|
||||
// snipped
|
||||
tidy: true,
|
||||
tidy_checks: [
|
||||
"android-*",
|
||||
"cert-*",
|
||||
"-cert-err34-c",
|
||||
"clang-analyzer-security*",
|
||||
// Disabling due to many unavoidable warnings from POSIX API usage.
|
||||
"-google-runtime-int",
|
||||
],
|
||||
}
|
||||
```
|
||||
That means in normal builds, even without `WITH_TIDY=1`,
|
||||
the modules that use `bpf_defaults` will run clang-tidy
|
||||
over C/C++ source files with the given `tidy_checks`.
|
||||
Note that `clang-analyzer-security*` is included in `tidy_checks`
|
||||
but not all `clang-analyzer-*` checks. Check `cert-err34-c` is
|
||||
disabled, although `cert-*` is selected.
|
||||
|
||||
Some modules might want to disable clang-tidy even when
|
||||
environment variable `WITH_TIDY=1` is set.
|
||||
Examples can be found in
|
||||
[system/netd/tests/Android.bp](https://android.googlesource.com/platform/system/netd/+/refs/heads/master/tests/Android.bp)
|
||||
```
|
||||
cc_test {
|
||||
name: "netd_integration_test",
|
||||
// snipped
|
||||
defaults: ["netd_defaults"],
|
||||
tidy: false, // cuts test build time by almost 1 minute
|
||||
```
|
||||
and in
|
||||
[bionic/tests/Android.bp](https://android.googlesource.com/platform/bionic/+/refs/heads/master/tests/Android.bp).
|
||||
```
|
||||
cc_test_library {
|
||||
name: "fortify_disabled_for_tidy",
|
||||
// snipped
|
||||
srcs: ["clang_fortify_tests.cpp"],
|
||||
tidy: false,
|
||||
}
|
||||
```
|
||||
|
||||
### `tidy_checks_as_errors`
|
||||
|
||||
The global tidy checks are enabled as warnings.
|
||||
If a C/C++ module wants to be free of certain clang-tidy warnings,
|
||||
it can chose those checks to be treated as errors.
|
||||
For example
|
||||
[system/core/libsysutils/Android.bp](https://android.googlesource.com/platform/system/core/+/refs/heads/master/libsysutils/Android.bp)
|
||||
has enabled clang-tidy explicitly, selected its own tidy checks,
|
||||
and set three groups of tidy checks as errors:
|
||||
```
|
||||
cc_library {
|
||||
name: "libsysutils",
|
||||
// snipped
|
||||
tidy: true,
|
||||
tidy_checks: [
|
||||
"-*",
|
||||
"cert-*",
|
||||
"clang-analyzer-security*",
|
||||
"android-*",
|
||||
],
|
||||
tidy_checks_as_errors: [
|
||||
"cert-*",
|
||||
"clang-analyzer-security*",
|
||||
"android-*",
|
||||
],
|
||||
// snipped
|
||||
}
|
||||
```
|
||||
|
||||
### `tidy_flags` and `tidy_disabled_srcs`
|
||||
|
||||
Extra clang-tidy flags can be passed with the `tidy_flags` property.
|
||||
|
||||
Some Android modules use the `tidy_flags` to pass "-warnings-as-errors="
|
||||
to clang-tidy. This usage should now be replaced with the
|
||||
`tidy_checks_as_errors` property.
|
||||
|
||||
Some other tidy flags examples are `-format-style=` and `-header-filter=`
|
||||
For example, in
|
||||
[art/odrefresh/Android.bp](https://android.googlesource.com/platform/art/+/refs/heads/master/odrefresh/Android.bp),
|
||||
we found
|
||||
```
|
||||
cc_defaults {
|
||||
name: "odrefresh-defaults",
|
||||
srcs: [
|
||||
"odrefresh.cc",
|
||||
"odr_common.cc",
|
||||
"odr_compilation_log.cc",
|
||||
"odr_fs_utils.cc",
|
||||
"odr_metrics.cc",
|
||||
"odr_metrics_record.cc",
|
||||
],
|
||||
// snipped
|
||||
generated_sources: [
|
||||
"apex-info-list-tinyxml",
|
||||
"art-apex-cache-info",
|
||||
"art-odrefresh-operator-srcs",
|
||||
],
|
||||
// snipped
|
||||
tidy: true,
|
||||
tidy_disabled_srcs: [":art-apex-cache-info"],
|
||||
tidy_flags: [
|
||||
"-format-style=file",
|
||||
"-header-filter=(art/odrefresh/|system/apex/)",
|
||||
],
|
||||
}
|
||||
```
|
||||
That means all modules with the `odrefresh-defaults` will
|
||||
have clang-tidy enabled, but not for generated source
|
||||
files in `art-apex-cache-info`.
|
||||
The clang-tidy is called with extra flags to specify the
|
||||
format-style and header-filter.
|
||||
|
||||
Note that the globally set default for header-filter is to
|
||||
include only the module directory. So, the default clang-tidy
|
||||
warnings for `art/odrefresh` modules will include source files
|
||||
under that directory. Now `odrefresh-defaults` is interested
|
||||
in seeing warnings from both `art/odrefresh/` and `system/apex/`
|
||||
and it redefines `-header-filter` in its `tidy_flags`.
|
||||
|
||||
|
||||
## Phony tidy-* targets
|
||||
|
||||
### The tidy-*directory* targets
|
||||
|
||||
Setting `WITH_TIDY=1` is easy to enable clang-tidy globally for any build.
|
||||
However, it adds extra compilation time.
|
||||
|
||||
For developers focusing on just one directory, they only want to compile
|
||||
their files with clang-tidy and wish to build other Android components as
|
||||
fast as possible. Changing the `WITH_TIDY=1` variable setting is also expensive
|
||||
since the build.ninja file will be regenerated due to any such variable change.
|
||||
|
||||
To manually select only some directories or modules to compile with clang-tidy,
|
||||
do not set the `WITH_TIDY=1` variable, but use the special `tidy-<directory>`
|
||||
phony target. For example, a person working on `system/libbase` can build
|
||||
Android quickly with
|
||||
```
|
||||
unset WITH_TIDY # Optional, not if you haven't set WITH_TIDY
|
||||
make droid tidy-system-libbase
|
||||
```
|
||||
|
||||
For any directory `d1/d2/d3`, a phony target tidy-d1-d2-d3 is generated
|
||||
if there is any C/C++ source file under `d1/d2/d3`.
|
||||
|
||||
Note that with `make droid tidy-system-libbase`, some C/C++ files
|
||||
that are not needed by the `droid` target will be passed to clang-tidy
|
||||
if they are under `system/libbase`. This is like a `checkbuild`
|
||||
under `system/libbase` to include all modules, but only C/C++
|
||||
files of those modules are compiled with clang-tidy.
|
||||
|
||||
### The tidy-soong target
|
||||
|
||||
A special `tidy-soong` target is defined to include all C/C++
|
||||
source files in *all* directories. This phony target is sometimes
|
||||
used to test if all source files compile with a new clang-tidy release.
|
||||
|
||||
### The tidy-*_subset targets
|
||||
|
||||
A *subset* of each tidy-* phony target is defined to reduce test time.
|
||||
Since any Android module, a C/C++ library or binary, can be built
|
||||
for many different *variants*, one C/C++ source file is usually
|
||||
compiled multiple times with different compilation flags.
|
||||
Many of such *variant* flags have little or no effect on clang-tidy
|
||||
checks. To reduce clang-tidy check time, a *subset* target like
|
||||
`tidy-soong_subset` or `tidy-system-libbase_subset` is generated
|
||||
to include only a subset, the first variant, of each module in
|
||||
the directory.
|
||||
|
||||
Hence, for C/C++ source code quality, instead of a long
|
||||
"make checkbuild", we can use "make tidy-soong_subset".
|
||||
|
||||
|
||||
## Limit clang-tidy runtime
|
||||
|
||||
Some Android modules have large files that take a long time to compile
|
||||
with clang-tidy, with or without the clang-analyzer checks.
|
||||
To limit clang-tidy time, an environment variable can be set as
|
||||
```base
|
||||
WITH_TIDY=1 TIDY_TIMEOUT=90 make
|
||||
```
|
||||
This 90-second limit is actually the default time limit
|
||||
in several Android continuous builds where `WITH_TIDY=1` and
|
||||
`CLANG_ANALYZER_CHECKS=1` are set.
|
||||
|
||||
## Capabilities for Android.bp and Android.mk
|
||||
|
||||
Some of the previously mentioned features are defined only
|
||||
for modules in Android.bp files, not for Android.mk modules yet.
|
||||
|
||||
* The global `WITH_TIDY=1` variable will enable clang-tidy for all C/C++
|
||||
modules in Android.bp or Android.mk files.
|
||||
|
||||
* The global `TIDY_TIMEOUT` variable is recognized by Android prebuilt
|
||||
clang-tidy, so it should work for any clang-tidy invocation.
|
||||
|
||||
* The clang-tidy module level properties are defined for Android.bp modules.
|
||||
For Android.mk modules, old `LOCAL_TIDY`, `LOCAL_TIDY_CHECKS`,
|
||||
`LOCAL_TIDY_FLAGS` work similarly, but it would be better to convert
|
||||
those modules to use Android.bp files.
|
||||
|
||||
* The `tidy-*` phony targets are only generated for Android.bp modules.
|
||||
Loading…
Reference in a new issue