Refactor how clang warning flags are set.

Previously, every gyp file that wanted to set clang warnings had to check for clang==1 and then set cflags and xcode_settings.WARNING_CFLAGS. Factor this out, so that targets only need to set clang_warning_flags for warnings that apply to all platforms. (Per-platform flags still need to be set manually.) This removes existing duplication from gyp files, and prevents adding more duplication when trying to add the same warning flags for clang/win. BUG=82385 R=hans@chromium.org, scottmg@chromium.org TBR=various owners Review URL: https://codereview.chromium.org/437543007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287092 0039d316-1c4b-4281-b951-d872f2087c98
markYoungH · Aug 1, 2014 · dcbc32c · dcbc32c
1 parent 83d8f43
commit dcbc32c
Show file tree

Hide file tree

Showing 13 changed files with 154 additions and 198 deletions.
diff --git a/breakpad/breakpad.gyp b/breakpad/breakpad.gyp
@@ -580,6 +580,12 @@
             'breakpad_processor_support',
             'linux_dumper_unittest_helper',
           ],
+          'variables': {
+            'clang_warning_flags': [
+              # See http://crbug.com/138571#c18
+              '-Wno-unused-value',
+            ],
+          },
 
           'sources': [
             'linux/breakpad_googletest_includes.h',
@@ -616,12 +622,6 @@
             '.',
           ],
           'conditions': [
-            [ 'clang == 1', {
-              'cflags': [
-                # See http://crbug.com/138571#c18
-                '-Wno-unused-value',
-              ],
-            }],
             ['OS=="android"', {
               'libraries': [
                 '-llog',

diff --git a/breakpad/breakpad_tools.gypi b/breakpad/breakpad_tools.gypi
@@ -7,13 +7,10 @@
     ['OS=="android"', {
       'toolsets': ['host'],
     }],
-    ['clang==1', {
-      'cflags': ['-Wno-tautological-constant-out-of-range-compare'],
-      'xcode_settings': {
-        'WARNING_CFLAGS': ['-Wno-tautological-constant-out-of-range-compare'],
-      },
-    }],
   ],
+  'variables': {
+    'clang_warning_flags': ['-Wno-tautological-constant-out-of-range-compare'],
+  },
   'include_dirs': [
     'src',
     'src/third_party',

diff --git a/build/common.gypi b/build/common.gypi
@@ -2400,7 +2400,40 @@
           'host_os%': '<(host_os)',  # See comment above chromium_code.
         }],
       ],
+      'clang_warning_flags': [
+        '-Wheader-hygiene',
+
+        # Don't die on dtoa code that uses a char as an array index.
+        # This is required solely for base/third_party/dmg_fp/dtoa.cc.
+        '-Wno-char-subscripts',
+
+        # TODO(thakis): This used to be implied by -Wno-unused-function,
+        # which we no longer use. Check if it makes sense to remove
+        # this as well. http://crbug.com/316352
+        '-Wno-unneeded-internal-declaration',
+
+        # Warns on switches on enums that cover all enum values but
+        # also contain a default: branch. Chrome is full of that.
+        '-Wno-covered-switch-default',
+
+        # Warns when a const char[] is converted to bool.
+        '-Wstring-conversion',
+
+        # C++11-related flags:
+
+        # This warns on using ints as initializers for floats in
+        # initializer lists (e.g. |int a = f(); CGSize s = { a, a };|),
+        # which happens in several places in chrome code. Not sure if
+        # this is worth fixing.
+        '-Wno-c++11-narrowing',
+
+        # Clang considers the `register` keyword as deprecated, but e.g.
+        # code generated by flex (used in angle) contains that keyword.
+        # http://crbug.com/255186
+        '-Wno-deprecated-register',
+      ],
     },
+    'includes': [ 'set_clang_warning_flags.gypi', ],
     'defines': [
       # Don't use deprecated V8 APIs anywhere.
       'V8_DEPRECATION_WARNINGS',
@@ -2873,18 +2906,7 @@
         'defines': ['OS_CHROMEOS=1'],
       }],
       ['enable_wexit_time_destructors==1', {
-        'conditions': [
-          [ 'clang==1', {
-            'cflags': [
-              '-Wexit-time-destructors',
-            ],
-            'xcode_settings': {
-              'WARNING_CFLAGS': [
-                '-Wexit-time-destructors',
-              ],
-            },
-          }],
-        ],
+        'variables': { 'clang_warning_flags': ['-Wexit-time-destructors']},
       }],
       ['chromium_code==0', {
         'conditions': [
@@ -3865,38 +3887,8 @@
           }],
           ['clang==1', {
             'cflags': [
-              '-Wheader-hygiene',
-
-              # Don't die on dtoa code that uses a char as an array index.
-              '-Wno-char-subscripts',
-
-              # TODO(thakis): This used to be implied by -Wno-unused-function,
-              # which we no longer use. Check if it makes sense to remove
-              # this as well. http://crbug.com/316352
-              '-Wno-unneeded-internal-declaration',
-
-              # Warns on switches on enums that cover all enum values but
-              # also contain a default: branch. Chrome is full of that.
-              '-Wno-covered-switch-default',
-
-              # Warns when a const char[] is converted to bool.
-              '-Wstring-conversion',
-
-              # C++11-related flags:
-
-              # This warns on using ints as initializers for floats in
-              # initializer lists (e.g. |int a = f(); CGSize s = { a, a };|),
-              # which happens in several places in chrome code. Not sure if
-              # this is worth fixing.
-              '-Wno-c++11-narrowing',
-
               # TODO(thakis): Remove, http://crbug.com/263960
               '-Wno-reserved-user-defined-literal',
-
-              # Clang considers the `register` keyword as deprecated, but e.g.
-              # code generated by flex (used in angle) contains that keyword.
-              # http://crbug.com/255186
-              '-Wno-deprecated-register',
             ],
             'cflags_cc': [
               # See the comment in the Mac section for what it takes to move
@@ -4675,35 +4667,6 @@
               'CLANG_WARN_OBJC_MISSING_PROPERTY_SYNTHESIS': 'YES',
               'GCC_VERSION': 'com.apple.compilers.llvm.clang.1_0',
               'WARNING_CFLAGS': [
-                '-Wheader-hygiene',
-
-                # This warns on using ints as initializers for floats in
-                # initializer lists (e.g. |int a = f(); CGSize s = { a, a };|),
-                # which happens in several places in chrome code. Not sure if
-                # this is worth fixing.
-                '-Wno-c++11-narrowing',
-
-                # Don't die on dtoa code that uses a char as an array index.
-                # This is required solely for base/third_party/dmg_fp/dtoa.cc.
-                '-Wno-char-subscripts',
-
-                # TODO(thakis): This used to be implied by -Wno-unused-function,
-                # which we no longer use. Check if it makes sense to remove
-                # this as well. http://crbug.com/316352
-                '-Wno-unneeded-internal-declaration',
-
-                # Warns on switches on enums that cover all enum values but
-                # also contain a default: branch. Chrome is full of that.
-                '-Wno-covered-switch-default',
-
-                # Warns when a const char[] is converted to bool.
-                '-Wstring-conversion',
-
-                # Clang considers the `register` keyword as deprecated, but
-                # e.g. code generated by flex (used in angle) contains that
-                # keyword. http://crbug.com/255186
-                '-Wno-deprecated-register',
-
                 # This warns on selectors from Cocoa headers (-length, -set).
                 # cfe-dev is currently discussing the merits of this warning.
                 # TODO(thakis): Reevaluate what to do with this, based one

diff --git a/build/set_clang_warning_flags.gypi b/build/set_clang_warning_flags.gypi
@@ -0,0 +1,44 @@
+# Copyright (c) 2014 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+# This file is meant to be included to set clang-specific compiler flags.
+# To use this the following variable can be defined:
+#   clang_warning_flags:       list: Compiler flags to pass to clang.
+#   clang_warning_flags_unset: list: Compiler flags to not pass to clang.
+#
+# Only use this in third-party code. In chromium_code, fix your code to not
+# warn instead!
+#
+# Note that the gypi file is included in target_defaults, so it does not need
+# to be explicitly included.
+#
+# Warning flags set by this will be used on all platforms. If you want to set
+# warning flags on only some platforms, you have to do so manually.
+#
+# To use this, create a gyp target with the following form:
+# {
+#   'target_name': 'my_target',
+#   'variables': {
+#     'clang_warning_flags': ['-Wno-awesome-warning'],
+#     'clang_warning_flags_unset': ['-Wpreviously-set-flag'],
+#   }
+# }
+
+{
+  'variables': {
+    'clang_warning_flags_unset%': [],  # Provide a default value.
+  },
+  'conditions': [
+    ['clang==1', {
+      # This uses >@ instead of @< to also see clang_warning_flags set in
+      # targets directly, not just the clang_warning_flags in target_defaults.
+      'cflags': [ '>@(clang_warning_flags)' ],
+      'cflags!': [ '>@(clang_warning_flags_unset)' ],
+      'xcode_settings': {
+        'WARNING_CFLAGS': ['>@(clang_warning_flags)'],
+        'WARNING_CFLAGS!': ['>@(clang_warning_flags_unset)'],
+      },
+    }],
+  ],
+}
diff --git a/skia/skia_chrome.gypi b/skia/skia_chrome.gypi
@@ -19,7 +19,13 @@
       'ext',
     ],
   },
-
+  'variables': {
+    # TODO(scottmg): http://crbug.com/177306
+    'clang_warning_flags_unset': [
+      # Don't warn about string->bool used in asserts.
+      '-Wstring-conversion',
+    ],
+  },
   'sources': [
     # Note: file list duplicated in GN build.
     'ext/analysis_canvas.cc',
@@ -99,18 +105,6 @@
         'skia_chrome_opts',
       ],
     }],
-    # TODO(scottmg): http://crbug.com/177306
-    ['clang==1', {
-      'xcode_settings': {
-        'WARNING_CFLAGS!': [
-          # Don't warn about string->bool used in asserts.
-          '-Wstring-conversion',
-        ],
-      },
-      'cflags!': [
-        '-Wstring-conversion',
-      ],
-    }],
     [ 'OS != "android" and (OS != "linux" or use_cairo==1)', {
       'sources!': [
         'ext/bitmap_platform_device_skia.cc',

diff --git a/skia/skia_common.gypi b/skia/skia_common.gypi
@@ -137,6 +137,11 @@
           'skia_support_pdf': 1,
         }],
       ],
+      # TODO(scottmg): http://crbug.com/177306
+      'clang_warning_flags': [
+        # Don't warn about string->bool used in asserts.
+        '-Wstring-conversion',
+      ]
     },
     'skia_support_gpu': '<(skia_support_gpu)',
     'skia_support_pdf': '<(skia_support_pdf)',

diff --git a/skia/skia_library.gypi b/skia/skia_library.gypi
@@ -358,18 +358,6 @@
         '../third_party/skia/src/utils/win/SkHRESULT.cpp',
       ],
     }],
-    # TODO(scottmg): http://crbug.com/177306
-    ['clang==1', {
-      'xcode_settings': {
-        'WARNING_CFLAGS!': [
-          # Don't warn about string->bool used in asserts.
-          '-Wstring-conversion',
-        ],
-      },
-      'cflags!': [
-        '-Wstring-conversion',
-      ],
-    }],
   ],
   'target_conditions': [
     # Pull in specific Mac files for iOS (which have been filtered out

diff --git a/third_party/libxml/libxml.gyp b/third_party/libxml/libxml.gyp
@@ -188,6 +188,21 @@
             # defines the macro FOO as 1.)
             'LIBXML_STATIC=',
           ],
+          'variables': {
+            'clang_warning_flags': [
+              # libxml passes `const unsigned char*` through `const char*`.
+              '-Wno-pointer-sign',
+              # pattern.c and uri.c both have an intentional
+              # `for (...);` / `while(...);` loop. I submitted a patch to
+              # move the `'` to its own line, but until that's landed
+              # suppress the warning:
+              '-Wno-empty-body',
+              # debugXML.c compares array 'arg' to NULL.
+              '-Wno-tautological-pointer-compare',
+              # See http://crbug.com/138571#c8
+              '-Wno-ignored-attributes',
+            ],
+          },
           'include_dirs': [
             '<(os_include)',
             '<(os_include)/include',
@@ -234,27 +249,6 @@
               'product_name': 'xml2',
             }],
             ['clang==1', {
-              'xcode_settings': {
-                'WARNING_CFLAGS': [
-                  # libxml passes `const unsigned char*` through `const char*`.
-                  '-Wno-pointer-sign',
-                  # pattern.c and uri.c both have an intentional
-                  # `for (...);` / `while(...);` loop. I submitted a patch to
-                  # move the `'` to its own line, but until that's landed
-                  # suppress the warning:
-                  '-Wno-empty-body',
-                  # debugXML.c compares array 'arg' to NULL.
-                  '-Wno-tautological-pointer-compare',
-                ],
-              },
-              'cflags': [
-                '-Wno-pointer-sign',
-                '-Wno-empty-body',
-                '-Wno-tautological-pointer-compare',
-
-                # See http://crbug.com/138571#c8
-                '-Wno-ignored-attributes',
-              ],
               'msvs_settings': {
                 'VCCLCompilerTool': {
                   'AdditionalOptions': [

diff --git a/third_party/libxslt/libxslt.gyp b/third_party/libxslt/libxslt.gyp
@@ -106,19 +106,14 @@
               '.',
             ],
           },
+          'variables': {
+            'clang_warning_flags': [
+              # libxslt stores a char[3] in a `const unsigned char*`.
+              '-Wno-pointer-sign',
+            ],
+          },
           'conditions': [
             ['OS!="win"', {'product_name': 'xslt'}],
-            ['clang == 1', {
-              'xcode_settings': {
-                'WARNING_CFLAGS': [
-                  # libxslt stores a char[3] in a `const unsigned char*`.
-                  '-Wno-pointer-sign',
-                ],
-              },
-              'cflags': [
-                '-Wno-pointer-sign',
-              ],
-            }],
           ],
         }],
       ],