Bug 1403346 - Implement ALLOW_COMPILER_WARNINGS as a template. draft
authorChris Manchester <cmanchester@mozilla.com>
Wed, 25 Oct 2017 15:12:09 -0700
changeset 686522 805c18b2e1a789630630b78910473a4ebd8439f2
parent 686521 05476293b73ed5bd87049b9a0aac73148c4ce728
child 686523 e63871b37a538773c4f6f15be916da697ae13e63
push id86198
push userbmo:cmanchester@mozilla.com
push dateWed, 25 Oct 2017 22:13:37 +0000
bugs1403346
milestone58.0a1
Bug 1403346 - Implement ALLOW_COMPILER_WARNINGS as a template. 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.