From 5cb580f407d2b1b639d7aeea8e1310740e55fe1c Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Mon, 26 Sep 2016 17:33:01 -0700 Subject: [PATCH 1/2] Start using "struct Objects" to store object Paths So that we can represent other files that get generated along with the objects, like the gcno coverage information, and per-file clang-tidy runs. Test: Soong's build.ninja identical before/after Change-Id: I5c553a153c436d5403549f62c73fe79c5f101779 --- cc/binary.go | 4 +-- cc/builder.go | 24 +++++++++++++++--- cc/cc.go | 22 ++++++++-------- cc/compiler.go | 10 ++++---- cc/library.go | 56 ++++++++++++++++++++--------------------- cc/linker.go | 2 +- cc/ndk_library.go | 6 ++--- cc/ndk_prebuilt.go | 6 ++--- cc/object.go | 10 ++++---- cc/prebuilt.go | 2 +- cc/toolchain_library.go | 6 ++--- 11 files changed, 81 insertions(+), 67 deletions(-) diff --git a/cc/binary.go b/cc/binary.go index e7d22c1ff..1e923a4f1 100644 --- a/cc/binary.go +++ b/cc/binary.go @@ -250,7 +250,7 @@ func (binary *binaryDecorator) linkerFlags(ctx ModuleContext, flags Flags) Flags } func (binary *binaryDecorator) link(ctx ModuleContext, - flags Flags, deps PathDeps, objFiles android.Paths) android.Path { + flags Flags, deps PathDeps, objs Objects) android.Path { fileName := binary.getStem(ctx) + flags.Toolchain.ExecutableSuffix() outputFile := android.PathForModuleOut(ctx, fileName) @@ -283,7 +283,7 @@ func (binary *binaryDecorator) link(ctx ModuleContext, linkerDeps = append(linkerDeps, deps.SharedLibsDeps...) linkerDeps = append(linkerDeps, deps.LateSharedLibsDeps...) - TransformObjToDynamicBinary(ctx, objFiles, sharedLibs, deps.StaticLibs, + TransformObjToDynamicBinary(ctx, objs.objFiles, sharedLibs, deps.StaticLibs, deps.LateStaticLibs, deps.WholeStaticLibs, linkerDeps, deps.CrtBegin, deps.CrtEnd, true, builderFlags, outputFile) diff --git a/cc/builder.go b/cc/builder.go index 60aad0b49..0006ed367 100644 --- a/cc/builder.go +++ b/cc/builder.go @@ -182,11 +182,27 @@ type builderFlags struct { stripAddGnuDebuglink bool } +type Objects struct { + objFiles android.Paths +} + +func (a Objects) Copy() Objects { + return Objects{ + objFiles: append(android.Paths{}, a.objFiles...), + } +} + +func (a Objects) Append(b Objects) Objects { + return Objects{ + objFiles: append(a.objFiles, b.objFiles...), + } +} + // Generate rules for compiling multiple .c, .cpp, or .S files to individual .o files func TransformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles android.Paths, - flags builderFlags, deps android.Paths) (objFiles android.Paths) { + flags builderFlags, deps android.Paths) Objects { - objFiles = make(android.Paths, len(srcFiles)) + objFiles := make(android.Paths, len(srcFiles)) cflags := flags.globalFlags + " " + flags.cFlags + " " + flags.conlyFlags cppflags := flags.globalFlags + " " + flags.cFlags + " " + flags.cppFlags @@ -250,7 +266,9 @@ func TransformSourceToObj(ctx android.ModuleContext, subdir string, srcFiles and }) } - return objFiles + return Objects{ + objFiles: objFiles, + } } // Generate a rule for compiling multiple .o files to a static library (.a) diff --git a/cc/cc.go b/cc/cc.go index 33c9611c1..c2884ace4 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -76,8 +76,8 @@ type PathDeps struct { StaticLibs, LateStaticLibs, WholeStaticLibs android.Paths // Paths to .o files - ObjFiles android.Paths - WholeStaticLibObjFiles android.Paths + Objs Objects + WholeStaticLibObjs Objects // Paths to generated source files GeneratedSources android.Paths @@ -174,7 +174,7 @@ type compiler interface { appendCflags([]string) appendAsflags([]string) - compile(ctx ModuleContext, flags Flags, deps PathDeps) android.Paths + compile(ctx ModuleContext, flags Flags, deps PathDeps) Objects } type linker interface { @@ -183,7 +183,7 @@ type linker interface { linkerFlags(ctx ModuleContext, flags Flags) Flags linkerProps() []interface{} - link(ctx ModuleContext, flags Flags, deps PathDeps, objFiles android.Paths) android.Path + link(ctx ModuleContext, flags Flags, deps PathDeps, objs Objects) android.Path appendLdflags([]string) } @@ -440,16 +440,16 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { flags.GlobalFlags = append(flags.GlobalFlags, deps.Flags...) - var objFiles android.Paths + var objs Objects if c.compiler != nil { - objFiles = c.compiler.compile(ctx, flags, deps) + objs = c.compiler.compile(ctx, flags, deps) if ctx.Failed() { return } } if c.linker != nil { - outputFile := c.linker.link(ctx, flags, deps, objFiles) + outputFile := c.linker.link(ctx, flags, deps, objs) if ctx.Failed() { return } @@ -813,8 +813,7 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { } if tag == reuseObjTag { - depPaths.ObjFiles = append(depPaths.ObjFiles, - cc.compiler.(libraryInterface).reuseObjs()...) + depPaths.Objs = depPaths.Objs.Append(cc.compiler.(libraryInterface).reuseObjs()) return } @@ -868,10 +867,9 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { } ctx.AddMissingDependencies(missingDeps) } - depPaths.WholeStaticLibObjFiles = - append(depPaths.WholeStaticLibObjFiles, staticLib.objs()...) + depPaths.WholeStaticLibObjs = depPaths.WholeStaticLibObjs.Append(staticLib.objs()) case objDepTag: - ptr = &depPaths.ObjFiles + depPaths.Objs.objFiles = append(depPaths.Objs.objFiles, linkFile.Path()) case crtBeginDepTag: depPaths.CrtBegin = linkFile case crtEndDepTag: diff --git a/cc/compiler.go b/cc/compiler.go index 0184ee9f4..454be5e3b 100644 --- a/cc/compiler.go +++ b/cc/compiler.go @@ -350,7 +350,7 @@ func ndkPathDeps(ctx ModuleContext) android.Paths { return nil } -func (compiler *baseCompiler) compile(ctx ModuleContext, flags Flags, deps PathDeps) android.Paths { +func (compiler *baseCompiler) compile(ctx ModuleContext, flags Flags, deps PathDeps) Objects { pathDeps := deps.GeneratedHeaders pathDeps = append(pathDeps, ndkPathDeps(ctx)...) @@ -367,18 +367,18 @@ func (compiler *baseCompiler) compile(ctx ModuleContext, flags Flags, deps PathD compiler.deps = pathDeps // Compile files listed in c.Properties.Srcs into objects - objFiles := compileObjs(ctx, buildFlags, "", srcs, compiler.deps) + objs := compileObjs(ctx, buildFlags, "", srcs, compiler.deps) if ctx.Failed() { - return nil + return Objects{} } - return objFiles + return objs } // Compile a list of source files into objects a specified subdirectory func compileObjs(ctx android.ModuleContext, flags builderFlags, - subdir string, srcFiles, deps android.Paths) android.Paths { + subdir string, srcFiles, deps android.Paths) Objects { return TransformSourceToObj(ctx, subdir, srcFiles, flags, deps) } diff --git a/cc/library.go b/cc/library.go index 99a9b4886..db19d6658 100644 --- a/cc/library.go +++ b/cc/library.go @@ -160,7 +160,7 @@ type libraryDecorator struct { Properties LibraryProperties // For reusing static library objects for shared library - reuseObjFiles android.Paths + reuseObjects Objects // table-of-contents file to optimize out relinking when possible tocFile android.OptionalPath @@ -173,7 +173,7 @@ type libraryDecorator struct { wholeStaticMissingDeps []string // For whole_static_libs - objFiles android.Paths + objects Objects // Uses the module's name if empty, but can be overridden. Does not include // shlib suffix. @@ -251,31 +251,29 @@ func (library *libraryDecorator) linkerFlags(ctx ModuleContext, flags Flags) Fla return flags } -func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps PathDeps) android.Paths { - var objFiles android.Paths - - objFiles = library.baseCompiler.compile(ctx, flags, deps) - library.reuseObjFiles = objFiles +func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps PathDeps) Objects { + objs := library.baseCompiler.compile(ctx, flags, deps) + library.reuseObjects = objs buildFlags := flagsToBuilderFlags(flags) if library.static() { srcs := android.PathsForModuleSrc(ctx, library.Properties.Static.Srcs) - objFiles = append(objFiles, compileObjs(ctx, buildFlags, android.DeviceStaticLibrary, - srcs, library.baseCompiler.deps)...) + objs = objs.Append(compileObjs(ctx, buildFlags, android.DeviceStaticLibrary, + srcs, library.baseCompiler.deps)) } else { srcs := android.PathsForModuleSrc(ctx, library.Properties.Shared.Srcs) - objFiles = append(objFiles, compileObjs(ctx, buildFlags, android.DeviceSharedLibrary, - srcs, library.baseCompiler.deps)...) + objs = objs.Append(compileObjs(ctx, buildFlags, android.DeviceSharedLibrary, + srcs, library.baseCompiler.deps)) } - return objFiles + return objs } type libraryInterface interface { getWholeStaticMissingDeps() []string static() bool - objs() android.Paths - reuseObjs() android.Paths + objs() Objects + reuseObjs() Objects toc() android.OptionalPath // Returns true if the build options for the module have selected a static or shared build @@ -340,18 +338,18 @@ func (library *libraryDecorator) linkerDeps(ctx BaseModuleContext, deps Deps) De } func (library *libraryDecorator) linkStatic(ctx ModuleContext, - flags Flags, deps PathDeps, objFiles android.Paths) android.Path { + flags Flags, deps PathDeps, objs Objects) android.Path { - library.objFiles = append(android.Paths{}, deps.WholeStaticLibObjFiles...) - library.objFiles = append(library.objFiles, objFiles...) + library.objects = deps.WholeStaticLibObjs.Copy() + library.objects = library.objects.Append(objs) outputFile := android.PathForModuleOut(ctx, ctx.ModuleName()+library.Properties.VariantName+staticLibraryExtension) if ctx.Darwin() { - TransformDarwinObjToStaticLib(ctx, library.objFiles, flagsToBuilderFlags(flags), outputFile) + TransformDarwinObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile) } else { - TransformObjToStaticLib(ctx, library.objFiles, flagsToBuilderFlags(flags), outputFile) + TransformObjToStaticLib(ctx, library.objects.objFiles, flagsToBuilderFlags(flags), outputFile) } library.wholeStaticMissingDeps = ctx.GetMissingDependencies() @@ -362,7 +360,7 @@ func (library *libraryDecorator) linkStatic(ctx ModuleContext, } func (library *libraryDecorator) linkShared(ctx ModuleContext, - flags Flags, deps PathDeps, objFiles android.Paths) android.Path { + flags Flags, deps PathDeps, objs Objects) android.Path { var linkerDeps android.Paths @@ -455,7 +453,7 @@ func (library *libraryDecorator) linkShared(ctx ModuleContext, linkerDeps = append(linkerDeps, deps.SharedLibsDeps...) linkerDeps = append(linkerDeps, deps.LateSharedLibsDeps...) - TransformObjToDynamicBinary(ctx, objFiles, sharedLibs, + TransformObjToDynamicBinary(ctx, objs.objFiles, sharedLibs, deps.StaticLibs, deps.LateStaticLibs, deps.WholeStaticLibs, linkerDeps, deps.CrtBegin, deps.CrtEnd, false, builderFlags, outputFile) @@ -463,15 +461,15 @@ func (library *libraryDecorator) linkShared(ctx ModuleContext, } func (library *libraryDecorator) link(ctx ModuleContext, - flags Flags, deps PathDeps, objFiles android.Paths) android.Path { + flags Flags, deps PathDeps, objs Objects) android.Path { - objFiles = append(objFiles, deps.ObjFiles...) + objs = objs.Append(deps.Objs) var out android.Path if library.static() { - out = library.linkStatic(ctx, flags, deps, objFiles) + out = library.linkStatic(ctx, flags, deps, objs) } else { - out = library.linkShared(ctx, flags, deps, objFiles) + out = library.linkShared(ctx, flags, deps, objs) } library.exportIncludes(ctx, "-I") @@ -505,12 +503,12 @@ func (library *libraryDecorator) getWholeStaticMissingDeps() []string { return library.wholeStaticMissingDeps } -func (library *libraryDecorator) objs() android.Paths { - return library.objFiles +func (library *libraryDecorator) objs() Objects { + return library.objects } -func (library *libraryDecorator) reuseObjs() android.Paths { - return library.reuseObjFiles +func (library *libraryDecorator) reuseObjs() Objects { + return library.reuseObjects } func (library *libraryDecorator) toc() android.OptionalPath { diff --git a/cc/linker.go b/cc/linker.go index 09233385a..28572c69a 100644 --- a/cc/linker.go +++ b/cc/linker.go @@ -197,6 +197,6 @@ func (linker *baseLinker) linkerFlags(ctx ModuleContext, flags Flags) Flags { } func (linker *baseLinker) link(ctx ModuleContext, - flags Flags, deps PathDeps, objFiles android.Paths) android.Path { + flags Flags, deps PathDeps, objs Objects) android.Path { panic(fmt.Errorf("baseLinker doesn't know how to link")) } diff --git a/cc/ndk_library.go b/cc/ndk_library.go index 6b0c325ed..48fbf4ddf 100644 --- a/cc/ndk_library.go +++ b/cc/ndk_library.go @@ -189,7 +189,7 @@ func (c *stubDecorator) compilerInit(ctx BaseModuleContext) { ndkMigratedLibs = append(ndkMigratedLibs, name) } -func (c *stubDecorator) compile(ctx ModuleContext, flags Flags, deps PathDeps) android.Paths { +func (c *stubDecorator) compile(ctx ModuleContext, flags Flags, deps PathDeps) Objects { arch := ctx.Arch().ArchType.String() if !strings.HasSuffix(ctx.ModuleName(), ndkLibrarySuffix) { @@ -242,11 +242,11 @@ func (stub *stubDecorator) linkerFlags(ctx ModuleContext, flags Flags) Flags { } func (stub *stubDecorator) link(ctx ModuleContext, flags Flags, deps PathDeps, - objFiles android.Paths) android.Path { + objs Objects) android.Path { linkerScriptFlag := "-Wl,--version-script," + stub.versionScriptPath.String() flags.LdFlags = append(flags.LdFlags, linkerScriptFlag) - return stub.libraryDecorator.link(ctx, flags, deps, objFiles) + return stub.libraryDecorator.link(ctx, flags, deps, objs) } func (stub *stubDecorator) install(ctx ModuleContext, path android.Path) { diff --git a/cc/ndk_prebuilt.go b/cc/ndk_prebuilt.go index 106c9b51f..46b383b98 100644 --- a/cc/ndk_prebuilt.go +++ b/cc/ndk_prebuilt.go @@ -79,7 +79,7 @@ func ndkPrebuiltObjectFactory() (blueprint.Module, []interface{}) { } func (c *ndkPrebuiltObjectLinker) link(ctx ModuleContext, flags Flags, - deps PathDeps, objFiles android.Paths) android.Path { + deps PathDeps, objs Objects) android.Path { // A null build step, but it sets up the output path. if !strings.HasPrefix(ctx.ModuleName(), "ndk_crt") { ctx.ModuleErrorf("NDK prebuilts must have an ndk_crt prefixed name") @@ -115,7 +115,7 @@ func ndkPrebuiltLibraryFactory() (blueprint.Module, []interface{}) { } func (ndk *ndkPrebuiltLibraryLinker) link(ctx ModuleContext, flags Flags, - deps PathDeps, objFiles android.Paths) android.Path { + deps PathDeps, objs Objects) android.Path { // A null build step, but it sets up the output path. ndk.exportIncludes(ctx, "-isystem") @@ -181,7 +181,7 @@ func getNdkStlLibDir(ctx android.ModuleContext, toolchain config.Toolchain, stl } func (ndk *ndkPrebuiltStlLinker) link(ctx ModuleContext, flags Flags, - deps PathDeps, objFiles android.Paths) android.Path { + deps PathDeps, objs Objects) android.Path { // A null build step, but it sets up the output path. if !strings.HasPrefix(ctx.ModuleName(), "ndk_lib") { ctx.ModuleErrorf("NDK prebuilts must have an ndk_lib prefixed name") diff --git a/cc/object.go b/cc/object.go index 72fd55bd5..57cc8b055 100644 --- a/cc/object.go +++ b/cc/object.go @@ -70,16 +70,16 @@ func (*objectLinker) linkerFlags(ctx ModuleContext, flags Flags) Flags { } func (object *objectLinker) link(ctx ModuleContext, - flags Flags, deps PathDeps, objFiles android.Paths) android.Path { + flags Flags, deps PathDeps, objs Objects) android.Path { - objFiles = append(objFiles, deps.ObjFiles...) + objs = objs.Append(deps.Objs) var outputFile android.Path - if len(objFiles) == 1 { - outputFile = objFiles[0] + if len(objs.objFiles) == 1 { + outputFile = objs.objFiles[0] } else { output := android.PathForModuleOut(ctx, ctx.ModuleName()+objectExtension) - TransformObjsToObj(ctx, objFiles, flagsToBuilderFlags(flags), output) + TransformObjsToObj(ctx, objs.objFiles, flagsToBuilderFlags(flags), output) outputFile = output } diff --git a/cc/prebuilt.go b/cc/prebuilt.go index 55775b69a..4328df8a0 100644 --- a/cc/prebuilt.go +++ b/cc/prebuilt.go @@ -46,7 +46,7 @@ func (p *prebuiltLibraryLinker) linkerProps() []interface{} { } func (p *prebuiltLibraryLinker) link(ctx ModuleContext, - flags Flags, deps PathDeps, objFiles android.Paths) android.Path { + flags Flags, deps PathDeps, objs Objects) android.Path { // TODO(ccross): verify shared library dependencies if len(p.Prebuilt.Properties.Srcs) > 0 { p.libraryDecorator.exportIncludes(ctx, "-I") diff --git a/cc/toolchain_library.go b/cc/toolchain_library.go index 0097ca3ef..2f3c9a1a0 100644 --- a/cc/toolchain_library.go +++ b/cc/toolchain_library.go @@ -53,12 +53,12 @@ func toolchainLibraryFactory() (blueprint.Module, []interface{}) { } func (library *toolchainLibraryDecorator) compile(ctx ModuleContext, flags Flags, - deps PathDeps) android.Paths { - return nil + deps PathDeps) Objects { + return Objects{} } func (library *toolchainLibraryDecorator) link(ctx ModuleContext, - flags Flags, deps PathDeps, objFiles android.Paths) android.Path { + flags Flags, deps PathDeps, objs Objects) android.Path { libName := ctx.ModuleName() + staticLibraryExtension outputFile := android.PathForModuleOut(ctx, libName) From a03cf6d3221142a7d38fefb187728a084f0b0367 Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Mon, 26 Sep 2016 15:45:04 -0700 Subject: [PATCH 2/2] 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 --- Android.bp | 5 ++ android/config.go | 11 +++ android/variable.go | 3 + androidmk/cmd/androidmk/android.go | 4 ++ cc/binary.go | 1 + cc/builder.go | 70 +++++++++++++++---- cc/cc.go | 6 ++ cc/check.go | 28 ++++++++ cc/config/tidy.go | 106 +++++++++++++++++++++++++++++ cc/config/tidy_test.go | 41 +++++++++++ cc/library.go | 5 +- cc/makevars.go | 13 ++++ cc/tidy.go | 93 +++++++++++++++++++++++++ cc/util.go | 2 + 14 files changed, 372 insertions(+), 16 deletions(-) create mode 100644 cc/config/tidy.go create mode 100644 cc/config/tidy_test.go create mode 100644 cc/tidy.go diff --git a/Android.bp b/Android.bp index f9ba2c22b..74d9c0ac8 100644 --- a/Android.bp +++ b/Android.bp @@ -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", diff --git a/android/config.go b/android/config.go index 4d7e8df55..74cb56e88 100644 --- a/android/config.go +++ b/android/config.go @@ -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" } diff --git a/android/variable.go b/android/variable.go index a1a2bf009..4aff26e31 100644 --- a/android/variable.go +++ b/android/variable.go @@ -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"` diff --git a/androidmk/cmd/androidmk/android.go b/androidmk/cmd/androidmk/android.go index 1fac4bf88..6387ff1b8 100644 --- a/androidmk/cmd/androidmk/android.go +++ b/androidmk/cmd/androidmk/android.go @@ -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}, } diff --git a/cc/binary.go b/cc/binary.go index 1e923a4f1..b02943947 100644 --- a/cc/binary.go +++ b/cc/binary.go @@ -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, diff --git a/cc/builder.go b/cc/builder.go index 0006ed367..faa39d152 100644 --- a/cc/builder.go +++ b/cc/builder.go @@ -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, }, diff --git a/cc/cc.go b/cc/cc.go index c2884ace4..b6e98b1cc 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -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...) diff --git a/cc/check.go b/cc/check.go index 4e403a58b..340464e4b 100644 --- a/cc/check.go +++ b/cc/check.go @@ -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) + } + } +} diff --git a/cc/config/tidy.go b/cc/config/tidy.go new file mode 100644 index 000000000..dd3942167 --- /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 000000000..4ed8b231d --- /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 db19d6658..dcfc077d2 100644 --- a/cc/library.go +++ b/cc/library.go @@ -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, diff --git a/cc/makevars.go b/cc/makevars.go index 770e1d0d9..7f1063f3e 100644 --- a/cc/makevars.go +++ b/cc/makevars.go @@ -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, " ") +} diff --git a/cc/tidy.go b/cc/tidy.go new file mode 100644 index 000000000..68380ec70 --- /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 07296b9e9..31f0aec77 100644 --- a/cc/util.go +++ b/cc/util.go @@ -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, } }