summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Yasin Kilicdere <tyk@google.com> 2022-06-10 12:18:07 +0000
committer Yasin Kilicdere <tyk@google.com> 2022-06-10 12:18:07 +0000
commit5a8ce13c2af6bf95e2fcdf2c4cc4187c88a99ffe (patch)
treea7d85eeb677406751e68f6f4f644591e990986f1
parent2d481842b1be6f5bd14331258e10b7cee6d9e2e5 (diff)
Revert "Disallow -warnings-as-errors in tidy_flags"
This reverts commit 2d481842b1be6f5bd14331258e10b7cee6d9e2e5. Reason for revert: b/235569235#comment4 Change-Id: I5c87b1d5a1bdf0bad7f436c397688fe3fc20d04e
-rw-r--r--cc/tidy.go31
-rw-r--r--cc/tidy_test.go70
2 files changed, 24 insertions, 77 deletions
diff --git a/cc/tidy.go b/cc/tidy.go
index e8e17831c..ff49c64bf 100644
--- a/cc/tidy.go
+++ b/cc/tidy.go
@@ -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)
}
diff --git a/cc/tidy_test.go b/cc/tidy_test.go
index 5863a6c17..14b33b239 100644
--- a/cc/tidy_test.go
+++ b/cc/tidy_test.go
@@ -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