Bug 1403346 - Implement ALLOW_COMPILER_WARNINGS as a template. r=glandium
authorChris Manchester <cmanchester@mozilla.com>
Wed, 25 Oct 2017 15:12:09 -0700
changeset 388309 0c470da05f01a572e7daa890408545bd65ce319a
parent 388308 3206a73af5455c96649d0192e1507437efc63729
child 388310 5f700fe50260b6ce9362506b02e1e5244b3c7686
push id32746
push useracraciun@mozilla.com
push dateThu, 26 Oct 2017 09:19:46 +0000
treeherdermozilla-central@485a03afaa23 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs1403346
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1403346 - Implement ALLOW_COMPILER_WARNINGS as a template. r=glandium MozReview-Commit-ID: 611XXi8hCKm
build/templates.mozbuild
python/mozbuild/mozbuild/compilation/database.py
python/mozbuild/mozbuild/frontend/context.py
python/mozbuild/mozbuild/test/backend/data/variable_passthru/moz.build
python/mozbuild/mozbuild/test/backend/test_recursivemake.py
python/mozbuild/mozbuild/test/frontend/data/allow-compiler-warnings/moz.build
python/mozbuild/mozbuild/test/frontend/data/allow-compiler-warnings/test1.c
python/mozbuild/mozbuild/test/frontend/data/variable-passthru/moz.build
python/mozbuild/mozbuild/test/frontend/test_emitter.py
--- a/build/templates.mozbuild
+++ b/build/templates.mozbuild
@@ -151,16 +151,19 @@ def HostRustLibrary(name, features=None)
 @template
 def DisableStlWrapping():
     COMPILE_FLAGS['STL'] = []
 
 @template
 def NoVisibilityFlags():
     COMPILE_FLAGS['VISIBILITY'] = []
 
+@template
+def AllowCompilerWarnings():
+    COMPILE_FLAGS['WARNINGS_AS_ERRORS'] = []
 
 @template
 def RustTest(name, features=None):
     RUST_TEST = name
 
     if features:
         RUST_TEST_FEATURES = features
 
--- a/python/mozbuild/mozbuild/compilation/database.py
+++ b/python/mozbuild/mozbuild/compilation/database.py
@@ -58,35 +58,28 @@ class CompileDBBackend(CommonBackend):
 
         consumed = CommonBackend.consume_object(self, obj)
 
         if consumed:
             return True
 
         if isinstance(obj, DirectoryTraversal):
             self._envs[obj.objdir] = obj.config
-            for var in ('WARNINGS_AS_ERRORS',):
-                value = obj.config.substs.get(var)
-                if value:
-                    self._local_flags[obj.objdir][var] = value
 
         elif isinstance(obj, (Sources, GeneratedSources)):
             # For other sources, include each source file.
             for f in obj.files:
                 self._build_db_line(obj.objdir, obj.relativedir, obj.config, f,
                                     obj.canonical_suffix)
 
         elif isinstance(obj, VariablePassthru):
             for var in ('MOZBUILD_CFLAGS', 'MOZBUILD_CXXFLAGS',
                         'MOZBUILD_CMFLAGS', 'MOZBUILD_CMMFLAGS'):
                 if var in obj.variables:
                     self._local_flags[obj.objdir][var] = obj.variables[var]
-            if (obj.variables.get('ALLOW_COMPILER_WARNINGS') and
-                    'WARNINGS_AS_ERRORS' in self._local_flags[obj.objdir]):
-                del self._local_flags[obj.objdir]['WARNINGS_AS_ERRORS']
 
         elif isinstance(obj, PerSourceFlag):
             self._per_source_flags[obj.file_name].extend(obj.flags)
 
         elif isinstance(obj, ComputedFlags):
             for var, flags in obj.get_flags():
                 self._local_flags[obj.objdir]['COMPUTED_%s' % var] = flags
 
@@ -189,16 +182,15 @@ class CompileDBBackend(CommonBackend):
             value = cenv.substs.get(name)
             if not value:
                 return
             if isinstance(value, types.StringTypes):
                 value = value.split()
             db.extend(value)
 
         db.append('$(COMPUTED_%s)' % self.CFLAGS[canonical_suffix])
-        db.append('$(WARNINGS_AS_ERRORS)')
         db.append('$(MOZBUILD_%s)' % self.CFLAGS[canonical_suffix])
         if canonical_suffix == '.m':
             append_var('OS_COMPILE_CMFLAGS')
             db.append('$(MOZBUILD_CMFLAGS)')
         elif canonical_suffix == '.mm':
             append_var('OS_COMPILE_CMMFLAGS')
             db.append('$(MOZBUILD_CMMFLAGS)')
--- a/python/mozbuild/mozbuild/frontend/context.py
+++ b/python/mozbuild/mozbuild/frontend/context.py
@@ -337,30 +337,48 @@ class CompileFlags(ContextDerivedValue, 
              ('CXXFLAGS', 'CXX_LDFLAGS')),
             ('DEBUG', (context.config.substs['MOZ_DEBUG_FLAGS'].split() if
                        'MOZ_DEBUG_FLAGS' in context.config.substs else []),
              ('CFLAGS', 'CXXFLAGS', 'CXX_LDFLAGS', 'C_LDFLAGS')),
             ('OPTIMIZE', self._optimize_flags(),
              ('CFLAGS', 'CXXFLAGS', 'CXX_LDFLAGS', 'C_LDFLAGS')),
             ('FRAMEPTR', context.config.substs.get('MOZ_FRAMEPTR_FLAGS'),
              ('CFLAGS', 'CXXFLAGS', 'CXX_LDFLAGS', 'C_LDFLAGS')),
+            ('WARNINGS_AS_ERRORS', self._warnings_as_errors(),
+             ('CXXFLAGS', 'CFLAGS', 'CXX_LDFLAGS', 'C_LDFLAGS')),
         )
         self._known_keys = set(k for k, v, _ in self.flag_variables)
 
         # Providing defaults here doesn't play well with multiple templates
         # modifying COMPILE_FLAGS from the same moz.build, because the merge
         # done after the template runs can't tell which values coming from
         # a template were set and which were provided as defaults.
         template_name = getattr(context, 'template', None)
         if template_name in (None, 'Gyp'):
             dict.__init__(self, ((k, v if v is None else TypedList(unicode)(v))
                                  for k, v, _ in self.flag_variables))
         else:
             dict.__init__(self)
 
+    def _warnings_as_errors(self):
+        warnings_as_errors = self._context.config.substs.get('WARNINGS_AS_ERRORS')
+        if self._context.config.substs.get('MOZ_PGO'):
+            # Don't use warnings-as-errors in Windows PGO builds because it is suspected of
+            # causing problems in that situation. (See bug 437002.)
+            if self._context.config.substs['OS_ARCH'] == 'WINNT':
+                warnings_as_errors = None
+
+        if self._context.config.substs.get('CC_TYPE') == 'clang-cl':
+            # Don't use warnings-as-errors in clang-cl because it warns about many more
+            # things than MSVC does.
+            warnings_as_errors = None
+
+        if warnings_as_errors:
+            return [warnings_as_errors]
+
     def _optimize_flags(self):
         if not self._context.config.substs.get('MOZ_OPTIMIZE'):
             return []
         optimize_flags = None
         if self._context.config.substs.get('MOZ_PGO'):
             optimize_flags = self._context.config.substs.get('MOZ_PGO_OPTIMIZE_FLAGS')
         if not optimize_flags:
             # If MOZ_PGO_OPTIMIZE_FLAGS is empty we fall back to MOZ_OPTIMIZE_FLAGS.
--- a/python/mozbuild/mozbuild/test/backend/data/variable_passthru/moz.build
+++ b/python/mozbuild/mozbuild/test/backend/data/variable_passthru/moz.build
@@ -10,10 +10,8 @@ RCINCLUDE = 'bar.rc'
 DEFFILE = 'baz.def'
 
 CFLAGS += ['-fno-exceptions', '-w']
 CXXFLAGS += ['-fcxx-exceptions', '-option with spaces']
 LDFLAGS += ['-ld flag with spaces', '-x']
 HOST_CFLAGS += ['-funroll-loops', '-wall']
 HOST_CXXFLAGS += ['-funroll-loops-harder', '-wall-day-everyday']
 WIN32_EXE_LDFLAGS += ['-subsystem:console']
-
-ALLOW_COMPILER_WARNINGS = True
--- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
+++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ -311,19 +311,16 @@ class TestRecursiveMakeBackend(BackendTe
     def test_variable_passthru(self):
         """Ensure variable passthru is written out correctly."""
         env = self._consume('variable_passthru', RecursiveMakeBackend)
 
         backend_path = mozpath.join(env.topobjdir, 'backend.mk')
         lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
 
         expected = {
-            'ALLOW_COMPILER_WARNINGS': [
-                'ALLOW_COMPILER_WARNINGS := 1',
-            ],
             'RCFILE': [
                 'RCFILE := foo.rc',
             ],
             'RESFILE': [
                 'RESFILE := bar.res',
             ],
             'RCINCLUDE': [
                 'RCINCLUDE := bar.rc',
new file mode 100644
--- /dev/null
+++ b/python/mozbuild/mozbuild/test/frontend/data/allow-compiler-warnings/moz.build
@@ -0,0 +1,17 @@
+# Any copyright is dedicated to the Public Domain.
+# http://creativecommons.org/publicdomain/zero/1.0/
+
+@template
+def AllowCompilerWarnings():
+    COMPILE_FLAGS['WARNINGS_AS_ERRORS'] = []
+
+@template
+def Library(name):
+    '''Template for libraries.'''
+    LIBRARY_NAME = name
+
+Library('dummy')
+
+UNIFIED_SOURCES += ['test1.c']
+
+AllowCompilerWarnings()
\ No newline at end of file
new file mode 100644
--- a/python/mozbuild/mozbuild/test/frontend/data/variable-passthru/moz.build
+++ b/python/mozbuild/mozbuild/test/frontend/data/variable-passthru/moz.build
@@ -12,10 +12,8 @@ RCINCLUDE = 'bar.rc'
 DEFFILE = 'baz.def'
 
 CFLAGS += ['-fno-exceptions', '-w']
 CXXFLAGS += ['-fcxx-exceptions', '-include foo.h']
 LDFLAGS += ['-framework Foo', '-x']
 HOST_CFLAGS += ['-funroll-loops', '-wall']
 HOST_CXXFLAGS += ['-funroll-loops-harder', '-wall-day-everyday']
 WIN32_EXE_LDFLAGS += ['-subsystem:console']
-
-ALLOW_COMPILER_WARNINGS = True
--- a/python/mozbuild/mozbuild/test/frontend/test_emitter.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ -178,17 +178,16 @@ class TestEmitterBasic(unittest.TestCase
     def test_variable_passthru(self):
         reader = self.reader('variable-passthru')
         objs = self.read_topsrcdir(reader)
 
         self.assertEqual(len(objs), 1)
         self.assertIsInstance(objs[0], VariablePassthru)
 
         wanted = {
-            'ALLOW_COMPILER_WARNINGS': True,
             'NO_DIST_INSTALL': True,
             'RCFILE': 'foo.rc',
             'RESFILE': 'bar.res',
             'RCINCLUDE': 'bar.rc',
             'DEFFILE': 'baz.def',
             'MOZBUILD_CFLAGS': ['-fno-exceptions', '-w'],
             'MOZBUILD_CXXFLAGS': ['-fcxx-exceptions', '-include foo.h'],
             'MOZBUILD_LDFLAGS': ['-framework Foo', '-x', '-DELAYLOAD:foo.dll',
@@ -201,21 +200,24 @@ class TestEmitterBasic(unittest.TestCase
 
         variables = objs[0].variables
         maxDiff = self.maxDiff
         self.maxDiff = None
         self.assertEqual(wanted, variables)
         self.maxDiff = maxDiff
 
     def test_compile_flags(self):
-        reader = self.reader('compile-flags')
+        reader = self.reader('compile-flags', extra_substs={
+            'WARNINGS_AS_ERRORS': '-Werror',
+        })
         sources, lib, flags = self.read_topsrcdir(reader)
         self.assertIsInstance(flags, ComputedFlags)
         self.assertEqual(flags.flags['STL'], reader.config.substs['STL_FLAGS'])
         self.assertEqual(flags.flags['VISIBILITY'], reader.config.substs['VISIBILITY_FLAGS'])
+        self.assertEqual(flags.flags['WARNINGS_AS_ERRORS'], ['-Werror'])
 
     def test_compile_flags_validation(self):
         reader = self.reader('compile-flags-field-validation')
 
         with self.assertRaisesRegexp(BuildReaderError, 'Invalid value.'):
             self.read_topsrcdir(reader)
 
         reader = self.reader('compile-flags-type-validation')
@@ -279,16 +281,23 @@ class TestEmitterBasic(unittest.TestCase
         self.assertEqual(flags.flags['BASE_INCLUDES'],
                          ['-I%s' % reader.config.topsrcdir,
                           '-I%s' % reader.config.topobjdir])
         self.assertEqual(flags.flags['EXTRA_INCLUDES'],
                          ['-I%s/dist/include' % reader.config.topobjdir])
         self.assertEqual(flags.flags['LOCAL_INCLUDES'],
                          ['-I%s/subdir' % reader.config.topsrcdir])
 
+    def test_allow_compiler_warnings(self):
+        reader = self.reader('allow-compiler-warnings', extra_substs={
+            'WARNINGS_AS_ERRORS': '-Werror',
+        })
+        sources, lib, flags = self.read_topsrcdir(reader)
+        self.assertEqual(flags.flags['WARNINGS_AS_ERRORS'], [])
+
     def test_use_yasm(self):
         # When yasm is not available, this should raise.
         reader = self.reader('use-yasm')
         with self.assertRaisesRegexp(SandboxValidationError,
             'yasm is not available'):
             self.read_topsrcdir(reader)
 
         # When yasm is available, this should work.