From 950e0baf2af9e705515ced80328df40c2a604151 Mon Sep 17 00:00:00 2001 From: Yi Kong Date: Tue, 8 Oct 2019 02:27:17 -0700 Subject: [PATCH] Expand ClangExtraExternalCflags to non-Google vendor projects Some of the warnings are too common to fix/opt-out for non-Google projects. Also in the change, minor clean up of duplicated code. Test: presubmit Bug: 139945549 Change-Id: Ic176ef1f17133405851a79592b6bef5ccb403bd9 --- Android.bp | 1 + cc/compiler.go | 35 +++++++++++++++++++++++++++++++---- cc/compiler_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ cc/config/clang.go | 4 ++-- 4 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 cc/compiler_test.go diff --git a/Android.bp b/Android.bp index 8d0c1ea6a..c73c3dace 100644 --- a/Android.bp +++ b/Android.bp @@ -206,6 +206,7 @@ bootstrap_go_package { ], testSrcs: [ "cc/cc_test.go", + "cc/compiler_test.go", "cc/gen_test.go", "cc/genrule_test.go", "cc/library_test.go", diff --git a/cc/compiler.go b/cc/compiler.go index bb40a5bd2..671861b0b 100644 --- a/cc/compiler.go +++ b/cc/compiler.go @@ -17,6 +17,7 @@ package cc import ( "fmt" "path/filepath" + "regexp" "strconv" "strings" @@ -252,6 +253,7 @@ func addToModuleList(ctx ModuleContext, key android.OnceKey, module string) { // per-target values, module type values, and per-module Blueprints properties func (compiler *baseCompiler) compilerFlags(ctx ModuleContext, flags Flags, deps PathDeps) Flags { tc := ctx.toolchain() + modulePath := android.PathForModuleSrc(ctx).String() compiler.srcsBeforeGen = android.PathsForModuleSrcExcludes(ctx, compiler.Properties.Srcs, compiler.Properties.Exclude_srcs) compiler.srcsBeforeGen = append(compiler.srcsBeforeGen, deps.GeneratedSources...) @@ -289,8 +291,8 @@ func (compiler *baseCompiler) compilerFlags(ctx ModuleContext, flags Flags, deps if compiler.Properties.Include_build_directory == nil || *compiler.Properties.Include_build_directory { - flags.Local.CommonFlags = append(flags.Local.CommonFlags, "-I"+android.PathForModuleSrc(ctx).String()) - flags.Local.YasmFlags = append(flags.Local.YasmFlags, "-I"+android.PathForModuleSrc(ctx).String()) + flags.Local.CommonFlags = append(flags.Local.CommonFlags, "-I"+modulePath) + flags.Local.YasmFlags = append(flags.Local.YasmFlags, "-I"+modulePath) } if !(ctx.useSdk() || ctx.useVndk()) || ctx.Host() { @@ -381,7 +383,7 @@ func (compiler *baseCompiler) compilerFlags(ctx ModuleContext, flags Flags, deps "${config.CommonClangGlobalCflags}", fmt.Sprintf("${config.%sClangGlobalCflags}", hod)) - if strings.HasPrefix(android.PathForModuleSrc(ctx).String(), "external/") { + if isThirdParty(modulePath) { flags.Global.CommonFlags = append([]string{"${config.ClangExternalCflags}"}, flags.Global.CommonFlags...) } @@ -438,7 +440,7 @@ func (compiler *baseCompiler) compilerFlags(ctx ModuleContext, flags Flags, deps // vendor/device specific things), we could extend this to be a ternary // value. strict := true - if strings.HasPrefix(android.PathForModuleSrc(ctx).String(), "external/") { + if strings.HasPrefix(modulePath, "external/") { strict = false } @@ -580,3 +582,28 @@ func compileObjs(ctx android.ModuleContext, flags builderFlags, return TransformSourceToObj(ctx, subdir, srcFiles, flags, pathDeps, cFlagsDeps) } + +var thirdPartyDirPrefixExceptions = []*regexp.Regexp{ + regexp.MustCompile("^vendor/[^/]*google[^/]*/"), + regexp.MustCompile("^hardware/google/"), + regexp.MustCompile("^hardware/interfaces/"), + regexp.MustCompile("^hardware/libhardware[^/]*/"), + regexp.MustCompile("^hardware/ril/"), +} + +func isThirdParty(path string) bool { + thirdPartyDirPrefixes := []string{"external/", "vendor/", "hardware/"} + + for _, prefix := range thirdPartyDirPrefixes { + if strings.HasPrefix(path, prefix) { + for _, prefix := range thirdPartyDirPrefixExceptions { + if prefix.MatchString(path) { + return false + } + } + break + } + } + + return true +} diff --git a/cc/compiler_test.go b/cc/compiler_test.go new file mode 100644 index 000000000..c301388ae --- /dev/null +++ b/cc/compiler_test.go @@ -0,0 +1,43 @@ +// Copyright 2019 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 ( + "testing" +) + +func TestIsThirdParty(t *testing.T) { + shouldFail := []string{ + "external/foo/", + "vendor/bar/", + "hardware/underwater_jaguar/", + } + shouldPass := []string{ + "vendor/google/cts/", + "hardware/google/pixel", + "hardware/interfaces/camera", + "hardware/ril/supa_ril", + } + for _, path := range shouldFail { + if !isThirdParty(path) { + t.Errorf("Expected %s to be considered third party", path) + } + } + for _, path := range shouldPass { + if isThirdParty(path) { + t.Errorf("Expected %s to *not* be considered third party", path) + } + } +} diff --git a/cc/config/clang.go b/cc/config/clang.go index 030a0768a..eddc34104 100644 --- a/cc/config/clang.go +++ b/cc/config/clang.go @@ -163,8 +163,8 @@ func init() { "-Wno-tautological-type-limit-compare", }, " ")) - // Extra cflags for projects under external/ directory to disable warnings that are infeasible - // to fix in all the external projects and their upstream repos. + // Extra cflags for external third-party projects to disable warnings that + // are infeasible to fix in all the external projects and their upstream repos. pctx.StaticVariable("ClangExtraExternalCflags", strings.Join([]string{ "-Wno-enum-compare", "-Wno-enum-compare-switch",