tequilaOS/platform_build_soong
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Revert "Disallow -warnings-as-errors in tidy_flags"

Browse source 
This reverts commit 2d481842b1.

Reason for revert: b/235569235#comment4

Change-Id: I5c87b1d5a1bdf0bad7f436c397688fe3fc20d04e
This commit is contained in:
Yasin Kilicdere 2022-06-10 12:18:07 +00:00
parent 2d481842b1
commit 5a8ce13c2a
2 changed files with 24 additions and 77 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
cc/tidy.go
View file

@ -188,14 +188,31 @@ func (tidy *tidyFeature) flags(ctx ModuleContext, flags Flags) Flags {
tidyChecks = tidyChecks + ",-cert-err33-c"
flags.TidyFlags = append(flags.TidyFlags, tidyChecks)
// Embedding -warnings-as-errors in tidy_flags is error-prone.
// It should be replaced with the tidy_checks_as_errors list.
for _, s := range flags.TidyFlags {
if strings.Contains(s, "-warnings-as-errors=") {
ctx.PropertyErrorf("tidy_flags", "should not contain -warnings-as-errors, use tidy_checks_as_errors instead")
if ctx.Config().IsEnvTrue("WITH_TIDY") {
// WITH_TIDY=1 enables clang-tidy globally. There could be many unexpected
// warnings from new checks and many local tidy_checks_as_errors and
// -warnings-as-errors can break a global build.
// So allow all clang-tidy warnings.
inserted := false
for i, s := range flags.TidyFlags {
if strings.Contains(s, "-warnings-as-errors=") {
// clang-tidy accepts only one -warnings-as-errors
// replace the old one
re := regexp.MustCompile(`'?-?-warnings-as-errors=[^ ]* *`)
newFlag := re.ReplaceAllString(s, "")
if newFlag == "" {
flags.TidyFlags[i] = "-warnings-as-errors=-*"
} else {
flags.TidyFlags[i] = newFlag + " -warnings-as-errors=-*"
}
inserted = true
break
}
}
}
if len(tidy.Properties.Tidy_checks_as_errors) > 0 {
if !inserted {
flags.TidyFlags = append(flags.TidyFlags, "-warnings-as-errors=-*")
}
} else if len(tidy.Properties.Tidy_checks_as_errors) > 0 {
tidyChecksAsErrors := "-warnings-as-errors=" + strings.Join(esc(ctx, "tidy_checks_as_errors", tidy.Properties.Tidy_checks_as_errors), ",")
flags.TidyFlags = append(flags.TidyFlags, tidyChecksAsErrors)
}

70
cc/tidy_test.go
View file

@ -22,76 +22,6 @@ import (
"android/soong/android"
)
func TestTidyFlagsWarningsAsErrors(t *testing.T) {
// The "tidy_flags" property should not contain -warnings-as-errors.
testCases := []struct {
libName, bp string
errorMsg string // a negative test; must have error message
flags []string // must have substrings in tidyFlags
noFlags []string // must not have substrings in tidyFlags
}{
{
"libfoo1",
`cc_library_shared { // no warnings-as-errors, good tidy_flags
name: "libfoo1",
srcs: ["foo.c"],
tidy_flags: ["-header-filter=dir1/"],
}`,
"",
[]string{"-header-filter=dir1/"},
[]string{"-warnings-as-errors"},
},
{
"libfoo2",
`cc_library_shared { // good use of tidy_checks_as_errors
name: "libfoo2",
srcs: ["foo.c"],
tidy_checks_as_errors: ["xyz-*", "abc"],
tidy_flags: ["-header-filter=dir2/"],
}`,
"",
[]string{"-header-filter=dir2/", "-warnings-as-errors='xyz-*',abc"},
[]string{},
},
{
"libfoo3",
`cc_library_shared { // bad use of -warnings-as-errors in tidy_flags
name: "libfoo3",
srcs: ["foo.c"],
tidy_flags: [
"-header-filters=.*",
"-warnings-as-errors=xyz-*",
],
}`,
`module "libfoo3" .*: tidy_flags: should not contain -warnings-as-errors,` +
` use tidy_checks_as_errors instead`,
[]string{},
[]string{},
},
}
for _, test := range testCases {
if test.errorMsg != "" {
testCcError(t, test.errorMsg, test.bp)
continue
}
variant := "android_arm64_armv8-a_shared"
ctx := testCc(t, test.bp)
t.Run("caseTidyFlags", func(t *testing.T) {
flags := ctx.ModuleForTests(test.libName, variant).Rule("clangTidy").Args["tidyFlags"]
for _, flag := range test.flags {
if !strings.Contains(flags, flag) {
t.Errorf("tidyFlags for %s does not contain %s.", test.libName, flag)
}
}
for _, flag := range test.noFlags {
if strings.Contains(flags, flag) {
t.Errorf("tidyFlags for %s should not contain %s.", test.libName, flag)
}
}
})
}
}
func TestTidyChecks(t *testing.T) {
// The "tidy_checks" property defines additional checks appended
// to global default. But there are some checks disabled after