StrixOS/build_soong
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add clang-tidy support

Browse Source 
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
This commit is contained in:
Dan Willemsen
2016-09-26 15:45:04 -07:00
parent 5cb580f407
commit a03cf6d322
14 changed files with 372 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
Android.bp
View File

@@ -95,6 +95,7 @@ bootstrap_go_package {
srcs: [
"cc/config/clang.go",
"cc/config/global.go",
"cc/config/tidy.go",
"cc/config/toolchain.go",
"cc/config/arm_device.go",
@@ -108,6 +109,9 @@ bootstrap_go_package {
"cc/config/x86_linux_host.go",
"cc/config/x86_windows_host.go",
],
testSrcs: [
"cc/config/tidy_test.go",
],
}
bootstrap_go_package {
@@ -134,6 +138,7 @@ bootstrap_go_package {
"cc/sanitize.go",
"cc/stl.go",
"cc/strip.go",
"cc/tidy.go",
"cc/util.go",
"cc/compiler.go",

11
android/config.go
View File

@@ -384,6 +384,17 @@ func (c *config) UseGoma() bool {
return Bool(c.ProductVariables.UseGoma)
}
func (c *config) ClangTidy() bool {
return Bool(c.ProductVariables.ClangTidy)
}
func (c *config) TidyChecks() string {
if c.ProductVariables.TidyChecks == nil {
return ""
}
return *c.ProductVariables.TidyChecks
}
func (c *config) LibartImgHostBaseAddress() string {
return "0x60000000"
}

3
android/variable.go
View File

@@ -112,6 +112,9 @@ type productVariables struct {
UseGoma *bool `json:",omitempty"`
Debuggable *bool `json:",omitempty"`
ClangTidy *bool `json:",omitempty"`
TidyChecks *string `json:",omitempty"`
DevicePrefer32BitExecutables *bool `json:",omitempty"`
HostPrefer32BitExecutables *bool `json:",omitempty"`

4
androidmk/cmd/androidmk/android.go
View File

@@ -53,6 +53,9 @@ var standardProperties = map[string]struct {
"LOCAL_EXPORT_SHARED_LIBRARY_HEADERS": {"export_shared_lib_headers", bpparser.ListType},
"LOCAL_EXPORT_STATIC_LIBRARY_HEADERS": {"export_static_lib_headers", bpparser.ListType},
"LOCAL_INIT_RC": {"init_rc", bpparser.ListType},
"LOCAL_TIDY_FLAGS": {"tidy_flags", bpparser.ListType},
// TODO: This is comma-seperated, not space-separated
"LOCAL_TIDY_CHECKS": {"tidy_checks", bpparser.ListType},
"LOCAL_JAVA_RESOURCE_DIRS": {"java_resource_dirs", bpparser.ListType},
"LOCAL_JAVACFLAGS": {"javacflags", bpparser.ListType},
@@ -73,6 +76,7 @@ var standardProperties = map[string]struct {
"LOCAL_RTTI_FLAG": {"rtti", bpparser.BoolType},
"LOCAL_NO_STANDARD_LIBRARIES": {"no_standard_libraries", bpparser.BoolType},
"LOCAL_PACK_MODULE_RELOCATIONS": {"pack_relocations", bpparser.BoolType},
"LOCAL_TIDY": {"tidy", bpparser.BoolType},
"LOCAL_EXPORT_PACKAGE_RESOURCES": {"export_package_resources", bpparser.BoolType},
}

1
cc/binary.go
View File

@@ -282,6 +282,7 @@ func (binary *binaryDecorator) link(ctx ModuleContext,
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,

70
cc/builder.go
View File

@@ -150,6 +150,14 @@ var (
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 @@ type builderFlags struct {
libFlags string
yaccFlags string
protoFlags string
tidyFlags string
toolchain config.Toolchain
clang bool
tidy bool
stripKeepSymbols bool
stripKeepMiniDebugInfo bool
@@ -183,18 +193,21 @@ type builderFlags struct {
}
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 @@ func TransformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
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 @@ func TransformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
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 @@ func TransformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and
"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 @@ func TransformObjToStaticLib(ctx android.ModuleContext, objFiles android.Paths,
// 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 @@ func TransformDarwinObjToStaticLib(ctx android.ModuleContext, objFiles android.P
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 @@ func TransformDarwinObjToStaticLib(ctx android.ModuleContext, objFiles android.P
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,
},

6
cc/cc.go
View File

@@ -100,9 +100,11 @@ type Flags struct {
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 newBaseModule(hod android.HostOrDeviceSupported, multilib android.Multilib)
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 @@ func DefaultsFactory(props ...interface{}) (blueprint.Module, []interface{}) {
&SanitizeProperties{},
&StripProperties{},
&InstallerProperties{},
&TidyProperties{},
)
return android.InitDefaultsModule(module, module, props...)

28
cc/check.go
View File

@@ -105,3 +105,31 @@ func CheckBadHostLdlibs(ctx ModuleContext, prop string, flags []string) {
}
}
}
// 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)
}
}
}

106
cc/config/tidy.go Normal file
View File

@@ -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
}

41
cc/config/tidy_test.go Normal file
View File

@@ -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)
}
})
}
}

5
cc/library.go
View File

@@ -347,9 +347,9 @@ func (library *libraryDecorator) linkStatic(ctx ModuleContext,
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 @@ func (library *libraryDecorator) linkShared(ctx ModuleContext,
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,

13
cc/makevars.go
View File

@@ -36,6 +36,7 @@ func makeVarsProvider(ctx android.MakeVarsContext) {
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 @@ func makeVarsProvider(ctx android.MakeVarsContext) {
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 @@ func splitSystemIncludes(ctx android.MakeVarsContext, val string) (includes, sys
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, " ")
}

93
cc/tidy.go Normal file
View File

@@ -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
}

2
cc/util.go
View File

@@ -96,8 +96,10 @@ func flagsToBuilderFlags(in Flags) builderFlags {
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,
}
}