Add clang-tidy support
For every file which we can run clang-tidy (C/C++ clang-built), we add a
new build node that depends on the object file (since clang-tidy does
not export a depfile), and is depended on by the link step. This is
better than how we're doing it in make, since calling tidy can be turned
on or off without needing to rebuild the object files.
This does not attempt to port WITH_TIDY_ONLY from Make, since the way
that it works is broken (due to the lack of a depfile).
Bug: 32244182
Test: WITH_TIDY=true mmma -j bionic/libc
Test: ./soong (Setting ClangTidy: true)
Change-Id: I40bbb5bb00d292d72bf1c293b93080b5f9f6d8ea
diff --git a/cc/binary.go b/cc/binary.go
index 1e923a4..b029439 100644
--- a/cc/binary.go
+++ b/cc/binary.go
@@ -282,6 +282,7 @@
linkerDeps = append(linkerDeps, deps.SharedLibsDeps...)
linkerDeps = append(linkerDeps, deps.LateSharedLibsDeps...)
+ linkerDeps = append(linkerDeps, objs.tidyFiles...)
TransformObjToDynamicBinary(ctx, objs.objFiles, sharedLibs, deps.StaticLibs,
deps.LateStaticLibs, deps.WholeStaticLibs, linkerDeps, deps.CrtBegin, deps.CrtEnd, true,
diff --git a/cc/builder.go b/cc/builder.go
index 0006ed3..faa39d1 100644
--- a/cc/builder.go
+++ b/cc/builder.go
@@ -150,6 +150,14 @@
Restat: true,
},
"crossCompile")
+
+ clangTidy = pctx.AndroidStaticRule("clangTidy",
+ blueprint.RuleParams{
+ Command: "rm -f $out && ${config.ClangBin}/clang-tidy $tidyFlags $in -- $cFlags && touch $out",
+ CommandDeps: []string{"${config.ClangBin}/clang-tidy"},
+ Description: "tidy $out",
+ },
+ "cFlags", "tidyFlags")
)
func init() {
@@ -174,8 +182,10 @@
libFlags string
yaccFlags string
protoFlags string
+ tidyFlags string
toolchain config.Toolchain
clang bool
+ tidy bool
stripKeepSymbols bool
stripKeepMiniDebugInfo bool
@@ -183,18 +193,21 @@
}
type Objects struct {
- objFiles android.Paths
+ objFiles android.Paths
+ tidyFiles android.Paths
}
func (a Objects) Copy() Objects {
return Objects{
- objFiles: append(android.Paths{}, a.objFiles...),
+ objFiles: append(android.Paths{}, a.objFiles...),
+ tidyFiles: append(android.Paths{}, a.tidyFiles...),
}
}
func (a Objects) Append(b Objects) Objects {
return Objects{
- objFiles: append(a.objFiles, b.objFiles...),
+ objFiles: append(a.objFiles, b.objFiles...),
+ tidyFiles: append(a.tidyFiles, b.tidyFiles...),
}
}
@@ -203,6 +216,10 @@
flags builderFlags, deps android.Paths) Objects {
objFiles := make(android.Paths, len(srcFiles))
+ var tidyFiles android.Paths
+ if flags.tidy && flags.clang {
+ tidyFiles = make(android.Paths, 0, len(srcFiles))
+ }
cflags := flags.globalFlags + " " + flags.cFlags + " " + flags.conlyFlags
cppflags := flags.globalFlags + " " + flags.cFlags + " " + flags.cppFlags
@@ -223,11 +240,13 @@
var moduleCflags string
var ccCmd string
+ tidy := flags.tidy && flags.clang
switch srcFile.Ext() {
case ".S", ".s":
ccCmd = "gcc"
moduleCflags = asflags
+ tidy = false
case ".c":
ccCmd = "gcc"
moduleCflags = cflags
@@ -264,24 +283,45 @@
"ccCmd": ccCmd,
},
})
+
+ if tidy {
+ tidyFile := android.ObjPathWithExt(ctx, srcFile, subdir, "tidy")
+ tidyFiles = append(tidyFiles, tidyFile)
+
+ ctx.ModuleBuild(pctx, android.ModuleBuildParams{
+ Rule: clangTidy,
+ Output: tidyFile,
+ Input: srcFile,
+ // We must depend on objFile, since clang-tidy doesn't
+ // support exporting dependencies.
+ Implicit: objFile,
+ Args: map[string]string{
+ "cFlags": moduleCflags,
+ "tidyFlags": flags.tidyFlags,
+ },
+ })
+ }
+
}
return Objects{
- objFiles: objFiles,
+ objFiles: objFiles,
+ tidyFiles: tidyFiles,
}
}
// Generate a rule for compiling multiple .o files to a static library (.a)
func TransformObjToStaticLib(ctx android.ModuleContext, objFiles android.Paths,
- flags builderFlags, outputFile android.ModuleOutPath) {
+ flags builderFlags, outputFile android.ModuleOutPath, deps android.Paths) {
arCmd := gccCmd(flags.toolchain, "ar")
arFlags := "crsPD"
ctx.ModuleBuild(pctx, android.ModuleBuildParams{
- Rule: ar,
- Output: outputFile,
- Inputs: objFiles,
+ Rule: ar,
+ Output: outputFile,
+ Inputs: objFiles,
+ Implicits: deps,
Args: map[string]string{
"arFlags": arFlags,
"arCmd": arCmd,
@@ -294,7 +334,7 @@
// very small command line length limit, so we have to split the ar into multiple
// steps, each appending to the previous one.
func TransformDarwinObjToStaticLib(ctx android.ModuleContext, objFiles android.Paths,
- flags builderFlags, outputPath android.ModuleOutPath) {
+ flags builderFlags, outputPath android.ModuleOutPath, deps android.Paths) {
arFlags := "cqs"
@@ -303,8 +343,9 @@
dummyAr := android.PathForModuleOut(ctx, "dummy"+staticLibraryExtension)
ctx.ModuleBuild(pctx, android.ModuleBuildParams{
- Rule: emptyFile,
- Output: dummy,
+ Rule: emptyFile,
+ Output: dummy,
+ Implicits: deps,
})
ctx.ModuleBuild(pctx, android.ModuleBuildParams{
@@ -347,9 +388,10 @@
if in == "" {
ctx.Build(pctx, blueprint.BuildParams{
- Rule: darwinAr,
- Outputs: []string{out},
- Inputs: l,
+ Rule: darwinAr,
+ Outputs: []string{out},
+ Inputs: l,
+ Implicits: deps.Strings(),
Args: map[string]string{
"arFlags": arFlags,
},
diff --git a/cc/cc.go b/cc/cc.go
index c2884ac..b6e98b1 100644
--- a/cc/cc.go
+++ b/cc/cc.go
@@ -100,9 +100,11 @@
protoFlags []string // Flags that apply to proto source files
LdFlags []string // Flags that apply to linker command lines
libFlags []string // Flags to add libraries early to the link order
+ TidyFlags []string // Flags that apply to clang-tidy
Toolchain config.Toolchain
Clang bool
+ Tidy bool
RequiredInstructionSet string
DynamicLinker string
@@ -368,6 +370,9 @@
func newModule(hod android.HostOrDeviceSupported, multilib android.Multilib) *Module {
module := newBaseModule(hod, multilib)
+ module.features = []feature{
+ &tidyFeature{},
+ }
module.stl = &stl{}
module.sanitize = &sanitize{}
return module
@@ -948,6 +953,7 @@
&SanitizeProperties{},
&StripProperties{},
&InstallerProperties{},
+ &TidyProperties{},
)
return android.InitDefaultsModule(module, module, props...)
diff --git a/cc/check.go b/cc/check.go
index 4e403a5..340464e 100644
--- a/cc/check.go
+++ b/cc/check.go
@@ -105,3 +105,31 @@
}
}
}
+
+// Check for bad clang tidy flags
+func CheckBadTidyFlags(ctx ModuleContext, prop string, flags []string) {
+ for _, flag := range flags {
+ flag = strings.TrimSpace(flag)
+
+ if !strings.HasPrefix(flag, "-") {
+ ctx.PropertyErrorf(prop, "Flag `%s` must start with `-`", flag)
+ } else if strings.HasPrefix(flag, "-fix") {
+ ctx.PropertyErrorf(prop, "Flag `%s` is not allowed, since it could cause multiple writes to the same source file", flag)
+ } else if strings.HasPrefix(flag, "-checks=") {
+ ctx.PropertyErrorf(prop, "Flag `%s` is not allowed, use `tidy_checks` property instead", flag)
+ } else if strings.Contains(flag, " ") {
+ ctx.PropertyErrorf(prop, "Bad flag: `%s` is not an allowed multi-word flag. Should it be split into multiple flags?", flag)
+ }
+ }
+}
+
+// Check for bad clang tidy checks
+func CheckBadTidyChecks(ctx ModuleContext, prop string, checks []string) {
+ for _, check := range checks {
+ if strings.Contains(check, " ") {
+ ctx.PropertyErrorf("tidy_checks", "Check `%s` invalid, cannot contain spaces", check)
+ } else if strings.Contains(check, ",") {
+ ctx.PropertyErrorf("tidy_checks", "Check `%s` invalid, cannot contain commas. Split each entry into it's own string instead", check)
+ }
+ }
+}
diff --git a/cc/config/tidy.go b/cc/config/tidy.go
new file mode 100644
index 0000000..dd39421
--- /dev/null
+++ b/cc/config/tidy.go
@@ -0,0 +1,106 @@
+// Copyright 2016 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package config
+
+import (
+ "strings"
+)
+
+func init() {
+ // Most Android source files are not clang-tidy clean yet.
+ // Global tidy checks include only google*, performance*,
+ // and misc-macro-parentheses, but not google-readability*
+ // or google-runtime-references.
+ pctx.StaticVariable("TidyDefaultGlobalChecks", strings.Join([]string{
+ "-*",
+ "google*",
+ "misc-macro-parentheses",
+ "performance*",
+ "-google-readability*",
+ "-google-runtime-references",
+ }, ","))
+
+ // There are too many clang-tidy warnings in external and vendor projects.
+ // Enable only some google checks for these projects.
+ pctx.StaticVariable("TidyExternalVendorChecks", strings.Join([]string{
+ "-*",
+ "google*",
+ "-google-build-using-namespace",
+ "-google-default-arguments",
+ "-google-explicit-constructor",
+ "-google-readability*",
+ "-google-runtime-int",
+ "-google-runtime-references",
+ }, ","))
+
+ // Give warnings to header files only in selected directories.
+ // Do not give warnings to external or vendor header files, which contain too
+ // many warnings.
+ pctx.StaticVariable("TidyDefaultHeaderDirs", strings.Join([]string{
+ "art/",
+ "bionic/",
+ "bootable/",
+ "build/",
+ "cts/",
+ "dalvik/",
+ "developers/",
+ "development/",
+ "frameworks/",
+ "libcore/",
+ "libnativehelper/",
+ "system/",
+ }, "|"))
+}
+
+type PathBasedTidyCheck struct {
+ PathPrefix string
+ Checks string
+}
+
+const tidyDefault = "${config.TidyDefaultGlobalChecks}"
+const tidyExternalVendor = "${config.TidyExternalVendorChecks}"
+
+// This is a map of local path prefixes to the set of default clang-tidy checks
+// to be used.
+// The last matched local_path_prefix should be the most specific to be used.
+var DefaultLocalTidyChecks = []PathBasedTidyCheck{
+ {"external/", tidyExternalVendor},
+ {"external/google", tidyDefault},
+ {"external/webrtc", tidyDefault},
+ {"frameworks/compile/mclinker/", tidyExternalVendor},
+ {"hardware/qcom", tidyExternalVendor},
+ {"vendor/", tidyExternalVendor},
+ {"vendor/google", tidyDefault},
+ {"vendor/google_devices", tidyExternalVendor},
+}
+
+var reversedDefaultLocalTidyChecks = reverseTidyChecks(DefaultLocalTidyChecks)
+
+func reverseTidyChecks(in []PathBasedTidyCheck) []PathBasedTidyCheck {
+ ret := make([]PathBasedTidyCheck, len(in))
+ for i, check := range in {
+ ret[len(in)-i-1] = check
+ }
+ return ret
+}
+
+func TidyChecksForDir(dir string) string {
+ for _, pathCheck := range reversedDefaultLocalTidyChecks {
+ if strings.HasPrefix(dir, pathCheck.PathPrefix) {
+ return pathCheck.Checks
+ }
+ }
+ return tidyDefault
+}
diff --git a/cc/config/tidy_test.go b/cc/config/tidy_test.go
new file mode 100644
index 0000000..4ed8b23
--- /dev/null
+++ b/cc/config/tidy_test.go
@@ -0,0 +1,41 @@
+// Copyright 2016 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package config
+
+import (
+ "testing"
+)
+
+func TestTidyChecksForDir(t *testing.T) {
+ testCases := []struct {
+ input string
+ expected string
+ }{
+ {"foo/bar", tidyDefault},
+ {"vendor/foo/bar", tidyExternalVendor},
+ {"vendor/google", tidyDefault},
+ {"vendor/google/foo", tidyDefault},
+ {"vendor/google_devices/foo", tidyExternalVendor},
+ }
+
+ for _, testCase := range testCases {
+ t.Run(testCase.input, func(t *testing.T) {
+ output := TidyChecksForDir(testCase.input)
+ if output != testCase.expected {
+ t.Error("Output doesn't match expected", output, testCase.expected)
+ }
+ })
+ }
+}
diff --git a/cc/library.go b/cc/library.go
index db19d66..dcfc077 100644
--- a/cc/library.go
+++ b/cc/library.go
@@ -347,9 +347,9 @@
ctx.ModuleName()+library.Properties.VariantName+staticLibraryExtension)
if ctx.Darwin() {
- TransformDarwinObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile)
+ TransformDarwinObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile, objs.tidyFiles)
} else {
- TransformObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile)
+ TransformObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile, objs.tidyFiles)
}
library.wholeStaticMissingDeps = ctx.GetMissingDependencies()
@@ -452,6 +452,7 @@
linkerDeps = append(linkerDeps, deps.SharedLibsDeps...)
linkerDeps = append(linkerDeps, deps.LateSharedLibsDeps...)
+ linkerDeps = append(linkerDeps, objs.tidyFiles...)
TransformObjToDynamicBinary(ctx, objs.objFiles, sharedLibs,
deps.StaticLibs, deps.LateStaticLibs, deps.WholeStaticLibs,
diff --git a/cc/makevars.go b/cc/makevars.go
index 770e1d0..7f1063f 100644
--- a/cc/makevars.go
+++ b/cc/makevars.go
@@ -36,6 +36,7 @@
ctx.Strict("CLANG_CXX", "${config.ClangBin}/clang++")
ctx.Strict("LLVM_AS", "${config.ClangBin}/llvm-as")
ctx.Strict("LLVM_LINK", "${config.ClangBin}/llvm-link")
+ ctx.Strict("PATH_TO_CLANG_TIDY", "${config.ClangBin}/clang-tidy")
ctx.StrictSorted("CLANG_CONFIG_UNKNOWN_CFLAGS", strings.Join(config.ClangUnknownCflags, " "))
ctx.Strict("GLOBAL_CFLAGS_NO_OVERRIDE", "${config.NoOverrideGlobalCflags}")
@@ -52,6 +53,10 @@
ctx.Strict("DEFAULT_CPP_STD_VERSION", config.CppStdVersion)
ctx.Strict("DEFAULT_GCC_CPP_STD_VERSION", config.GccCppStdVersion)
+ ctx.Strict("DEFAULT_GLOBAL_TIDY_CHECKS", "${config.TidyDefaultGlobalChecks}")
+ ctx.Strict("DEFAULT_LOCAL_TIDY_CHECKS", joinLocalTidyChecks(config.DefaultLocalTidyChecks))
+ ctx.Strict("DEFAULT_TIDY_HEADER_DIRS", "${config.TidyDefaultHeaderDirs}")
+
includeFlags, err := ctx.Eval("${config.CommonGlobalIncludes} ${config.CommonGlobalSystemIncludes}")
if err != nil {
panic(err)
@@ -257,3 +262,11 @@
return includes, systemIncludes
}
+
+func joinLocalTidyChecks(checks []config.PathBasedTidyCheck) string {
+ rets := make([]string, len(checks))
+ for i, check := range config.DefaultLocalTidyChecks {
+ rets[i] = check.PathPrefix + ":" + check.Checks
+ }
+ return strings.Join(rets, " ")
+}
diff --git a/cc/tidy.go b/cc/tidy.go
new file mode 100644
index 0000000..68380ec
--- /dev/null
+++ b/cc/tidy.go
@@ -0,0 +1,93 @@
+// Copyright 2016 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cc
+
+import (
+ "strings"
+
+ "github.com/google/blueprint/proptools"
+
+ "android/soong/cc/config"
+)
+
+type TidyProperties struct {
+ // whether to run clang-tidy over C-like sources.
+ Tidy *bool
+
+ // Extra flags to pass to clang-tidy
+ Tidy_flags []string
+
+ // Extra checks to enable or disable in clang-tidy
+ Tidy_checks []string
+}
+
+type tidyFeature struct {
+ Properties TidyProperties
+}
+
+func (tidy *tidyFeature) props() []interface{} {
+ return []interface{}{&tidy.Properties}
+}
+
+func (tidy *tidyFeature) begin(ctx BaseModuleContext) {
+}
+
+func (tidy *tidyFeature) deps(ctx BaseModuleContext, deps Deps) Deps {
+ return deps
+}
+
+func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags {
+ // Check if tidy is explicitly disabled for this module
+ if tidy.Properties.Tidy != nil && !*tidy.Properties.Tidy {
+ return flags
+ }
+
+ // If not explicitly set, check the global tidy flag
+ if tidy.Properties.Tidy == nil && !ctx.AConfig().ClangTidy() {
+ return flags
+ }
+
+ // Clang-tidy requires clang
+ if !flags.Clang {
+ return flags
+ }
+
+ flags.Tidy = true
+
+ CheckBadTidyFlags(ctx, "tidy_flags", tidy.Properties.Tidy_flags)
+
+ esc := proptools.NinjaAndShellEscape
+
+ flags.TidyFlags = append(flags.TidyFlags, esc(tidy.Properties.Tidy_flags)...)
+ if len(flags.TidyFlags) == 0 {
+ headerFilter := "-header-filter=\"(" + ctx.ModuleDir() + "|${config.TidyDefaultHeaderDirs})\""
+ flags.TidyFlags = append(flags.TidyFlags, headerFilter)
+ }
+
+ tidyChecks := "-checks="
+ if checks := ctx.AConfig().TidyChecks(); len(checks) > 0 {
+ tidyChecks += checks
+ } else {
+ tidyChecks += config.TidyChecksForDir(ctx.ModuleDir())
+ }
+ if len(tidy.Properties.Tidy_checks) > 0 {
+ CheckBadTidyChecks(ctx, "tidy_checks", tidy.Properties.Tidy_checks)
+
+ tidyChecks = tidyChecks + "," + strings.Join(esc(tidy.Properties.Tidy_checks), ",")
+ }
+ flags.TidyFlags = append(flags.TidyFlags, tidyChecks)
+
+ return flags
+}
diff --git a/cc/util.go b/cc/util.go
index 07296b9..31f0aec 100644
--- a/cc/util.go
+++ b/cc/util.go
@@ -96,8 +96,10 @@
protoFlags: strings.Join(in.protoFlags, " "),
ldFlags: strings.Join(in.LdFlags, " "),
libFlags: strings.Join(in.libFlags, " "),
+ tidyFlags: strings.Join(in.TidyFlags, " "),
toolchain: in.Toolchain,
clang: in.Clang,
+ tidy: in.Tidy,
}
}