compilers: New class CompilerArgs derived from list()

The purpose of this class is to make it possible to sanely generate
compiler command-lines by ensuring that new arguments appended or added
to a list of arguments properly override previous arguments.
For instance:

>>> a = CompilerArgs(['-Lfoo', '-DBAR'])
>>> a += ['-Lgah', '-DTAZ']
>>> print(a)
['-Lgah', '-Lfoo', '-DBAR', '-DTAZ']

Arguments will be de-duped if it is safe to do so. Currently, this is
only done for -I and -L arguments (previous occurances are removed when
a new one is added) and arguments that once added cannot be overriden
such as -pipe are removed completely.

mesonbuild/backend/backends.py

@@ -360,7 +360,7 @@ class Backend:
         for dep in target.get_external_deps():
             # Cflags required by external deps might have UNIX-specific flags,
             # so filter them out if needed
-            commands += compiler.unix_compile_flags_to_native(dep.get_compile_args())
+            commands += compiler.unix_args_to_native(dep.get_compile_args())
             if isinstance(target, build.Executable):
                 commands += dep.get_exe_args()
 

mesonbuild/backend/ninjabackend.py

@@ -2071,7 +2071,7 @@ rule FORTRAN_DEP_HACK
         custom_target_libraries = self.get_custom_target_provided_libraries(target)
         commands += extra_args
         commands += custom_target_libraries
-        commands = linker.unix_link_flags_to_native(self.dedup_arguments(commands))
+        commands = linker.unix_args_to_native(self.dedup_arguments(commands))
         dep_targets = [self.get_dependency_filename(t) for t in dependencies]
         dep_targets += [os.path.join(self.environment.source_dir,
                                      target.subdir, t) for t in target.link_depends]

mesonbuild/backend/vs2010backend.py

@@ -687,14 +687,14 @@ class Vs2010Backend(backends.Backend):
                 file_args[l] += args
         for l, args in target.extra_args.items():
             if l in file_args:
-                file_args[l] += compiler.unix_compile_flags_to_native(args)
+                file_args[l] += compiler.unix_args_to_native(args)
         for l, comp in target.compilers.items():
             if l in file_args:
                 file_args[l] += comp.get_option_compile_args(self.environment.coredata.compiler_options)
         for d in target.get_external_deps():
             # Cflags required by external deps might have UNIX-specific flags,
             # so filter them out if needed
-            d_compile_args = compiler.unix_compile_flags_to_native(d.get_compile_args())
+            d_compile_args = compiler.unix_args_to_native(d.get_compile_args())
             for arg in d_compile_args:
                 if arg.startswith(('-D', '/D')):
                     define = arg[2:]
@@ -793,7 +793,7 @@ class Vs2010Backend(backends.Backend):
                 if isinstance(d, build.StaticLibrary):
                     for dep in d.get_external_deps():
                         extra_link_args += dep.get_link_args()
-        extra_link_args = compiler.unix_link_flags_to_native(extra_link_args)
+        extra_link_args = compiler.unix_args_to_native(extra_link_args)
         (additional_libpaths, additional_links, extra_link_args) = self.split_link_args(extra_link_args)
         if len(extra_link_args) > 0:
             extra_link_args.append('%(AdditionalOptions)')

mesonbuild/compilers.py

@@ -323,6 +323,175 @@ class RunResult:
         self.stdout = stdout
         self.stderr = stderr
 
+class CompilerArgs(list):
+    '''
+    Class derived from list() that manages a list of compiler arguments. Should
+    be used while constructing compiler arguments from various sources. Can be
+    operated with ordinary lists, so this does not need to be used everywhere.
+
+    All arguments must be inserted and stored in GCC-style (-lfoo, -Idir, etc)
+    and can converted to the native type of each compiler by using the
+    .to_native() method to which you must pass an instance of the compiler or
+    the compiler class.
+
+    New arguments added to this class (either with .append(), .extend(), or +=)
+    are added in a way that ensures that they override previous arguments.
+    For example:
+
+    >>> a = ['-Lfoo', '-lbar']
+    >>> a += ['-Lpho', '-lbaz']
+    >>> print(a)
+    ['-Lpho', '-Lfoo', '-lbar', '-lbaz']
+
+    Arguments will also be de-duped if they can be de-duped safely.
+
+    Note that because of all this, this class is not commutative and does not
+    preserve the order of arguments if it is safe to not. For example:
+    >>> ['-Ifoo', '-Ibar'] + ['-Ifez', '-Ibaz', '-Werror']
+    ['-Ifez', '-Ibaz', '-Ifoo', '-Ibar', '-Werror']
+    >>> ['-Ifez', '-Ibaz', '-Werror'] + ['-Ifoo', '-Ibar']
+    ['-Ifoo', '-Ibar', '-Ifez', '-Ibaz', '-Werror']
+
+    '''
+    # NOTE: currently this class is only for C-like compilers, but it can be
+    # extended to other languages easily. Just move the following to the
+    # compiler class and initialize when self.compiler is set.
+
+    # Arg prefixes that override by prepending instead of appending
+    prepend_prefixes = ('-I', '-L')
+    # Arg prefixes and args that must be de-duped by returning 2
+    dedup2_prefixes = ('-I', '-L', '-D')
+    dedup2_args = ()
+    # Arg prefixes and args that must be de-duped by returning 1
+    dedup1_prefixes = ()
+    dedup1_args = ('-c', '-S', '-E', '-pipe')
+    compiler = None
+
+    def _check_args(self, args):
+        cargs = []
+        if len(args) > 2:
+            raise TypeError("CompilerArgs() only accepts at most 2 arguments: "
+                            "The compiler, and optionally an initial list")
+        elif len(args) == 0:
+            return cargs
+        elif len(args) == 1:
+            if isinstance(args[0], (Compiler, StaticLinker)):
+                self.compiler = args[0]
+            else:
+                raise TypeError("you must pass a Compiler instance as one of "
+                                "the arguments")
+        elif len(args) == 2:
+            if isinstance(args[0], (Compiler, StaticLinker)):
+                self.compiler = args[0]
+                cargs = args[1]
+            elif isinstance(args[1], (Compiler, StaticLinker)):
+                cargs = args[0]
+                self.compiler = args[1]
+            else:
+                raise TypeError("you must pass a Compiler instance as one of "
+                                "the two arguments")
+        else:
+            raise AssertionError('Not reached')
+        return cargs
+
+    def __init__(self, *args):
+        super().__init__(self._check_args(args))
+
+    @classmethod
+    def _can_dedup(cls, arg):
+        '''
+        Returns whether the argument can be safely de-duped. This is dependent
+        on two things:
+
+        a) Whether an argument can be 'overriden' by a later argument.  For
+           example, -DFOO defines FOO and -UFOO undefines FOO. In this case, we
+           can safely remove the previous occurance and add a new one. The same
+           is true for include paths and library paths with -I and -L. For
+           these we return `2`. See `dedup2_prefixes` and `dedup2_args`.
+        b) Arguments that once specifie cannot be undone, such as `-c` or
+           `-pipe`. New instances of these can be completely skipped. For these
+           we return `1`. See `dedup1_prefixes` and `dedup1_args`.
+        c) Whether it matters where or how many times on the command-line
+           a particular argument is present. This can matter for symbol
+           resolution in static or shared libraries, so we cannot de-dup or
+           reorder them. For these we return `0`. This is the default.
+        '''
+        if arg.startswith(cls.dedup2_prefixes) or arg in cls.dedup2_args:
+            return 2
+        if arg.startswith(cls.dedup1_prefixes) or arg in cls.dedup1_args:
+            return 1
+        return 0
+
+    @classmethod
+    def _should_prepend(cls, arg):
+        if arg.startswith(cls.prepend_prefixes):
+            return True
+        return False
+
+    def to_native(self):
+        return self.compiler.unix_args_to_native(self)
+
+    def __add__(self, args):
+        new = CompilerArgs(self, self.compiler)
+        new += args
+        return new
+
+    def __iadd__(self, args):
+        '''
+        Add two CompilerArgs while taking into account overriding of arguments
+        and while preserving the order of arguments as much as possible
+        '''
+        pre = []
+        post = []
+        if not isinstance(args, list):
+            raise TypeError('can only concatenate list (not "{}") to list'.format(args))
+        for arg in args:
+            # If the argument can be de-duped, do it either by removing the
+            # previous occurance of it and adding a new one, or not adding the
+            # new occurance.
+            dedup = self._can_dedup(arg)
+            if dedup == 1:
+                # Argument already exists and adding a new instance is useless
+                if arg in self or arg in pre or arg in post:
+                    continue
+            if dedup == 2:
+                # Remove all previous occurances of the arg and add it anew
+                if arg in self:
+                    self.remove(arg)
+                if arg in pre:
+                    pre.remove(arg)
+                if arg in post:
+                    post.remove(arg)
+            if self._should_prepend(arg):
+                pre.append(arg)
+            else:
+                post.append(arg)
+        # Insert at the beginning
+        self[:0] = pre
+        # Append to the end
+        super().__iadd__(post)
+        return self
+
+    def __radd__(self, args):
+        new = CompilerArgs(args, self.compiler)
+        new += self
+        return new
+
+    def __mul__(self, args):
+        raise TypeError("can't multiply compiler arguments")
+
+    def __imul__(self, args):
+        raise TypeError("can't multiply compiler arguments")
+
+    def __rmul__(self, args):
+        raise TypeError("can't multiply compiler arguments")
+
+    def append(self, arg):
+        self.__iadd__([arg])
+
+    def extend(self, args):
+        self.__iadd__(args)
+
 class Compiler:
     def __init__(self, exelist, version):
         if isinstance(exelist, str):
@@ -412,11 +581,8 @@ class Compiler:
     def has_function(self, *args, **kwargs):
         raise EnvironmentException('Language %s does not support function checks.' % self.language)
 
-    def unix_link_flags_to_native(self, args):
+    @classmethod
-        "Always returns a copy that can be independently mutated"
+    def unix_args_to_native(cls, args):
-        return args[:]
-
-    def unix_compile_flags_to_native(self, args):
         "Always returns a copy that can be independently mutated"
         return args[:]
 
@@ -435,7 +601,7 @@ class Compiler:
                 self.language))
 
     def get_cross_extra_flags(self, environment, *, compile, link):
-        extra_flags = []
+        extra_flags = CompilerArgs(self)
         if self.is_cross and environment:
             if 'properties' in environment.cross_info.config:
                 lang_args_key = self.language + '_args'
@@ -474,7 +640,7 @@ class Compiler:
                 output = self._get_compile_output(tmpdirname, mode)
 
                 # Construct the compiler command-line
-                commands = self.get_exelist()
+                commands = CompilerArgs(self)
                 commands.append(srcname)
                 commands += extra_args
                 commands += self.get_always_args()
@@ -485,6 +651,8 @@ class Compiler:
                     commands += self.get_preprocess_only_args()
                 else:
                     commands += self.get_output_args(output)
+                # Generate full command-line with the exelist
+                commands = self.get_exelist() + commands.to_native()
                 mlog.debug('Running compile:')
                 mlog.debug('Working directory: ', tmpdirname)
                 mlog.debug('Command line: ', ' '.join(commands), '\n')
@@ -663,7 +831,7 @@ class CCompiler(Compiler):
         mlog.debug('Sanity testing ' + self.language + ' compiler:', ' '.join(self.exelist))
         mlog.debug('Is cross compiler: %s.' % str(self.is_cross))
 
-        extra_flags = []
+        extra_flags = CompilerArgs(self)
         source_name = os.path.join(work_dir, sname)
         binname = sname.rsplit('.', 1)[0]
         if self.is_cross:
@@ -742,51 +910,29 @@ class CCompiler(Compiler):
         }}'''
         return self.compiles(t.format(**fargs), env, extra_args, dependencies)
 
-    @staticmethod
-    def _override_args(args, override):
-        '''
-        Add @override to @args in such a way that arguments are overriden
-        correctly.
-
-        We want the include directories to be added first (since they are
-        chosen left-to-right) and all other arguments later (since they
-        override previous arguments or add to a list that's chosen
-        right-to-left).
-        '''
-        before_args = []
-        after_args = []
-        for arg in override:
-            if arg.startswith(('-I', '/I')):
-                before_args.append(arg)
-            else:
-                after_args.append(arg)
-        return before_args + args + after_args
-
     def compiles(self, code, env, extra_args=None, dependencies=None, mode='compile'):
         if extra_args is None:
             extra_args = []
-        if isinstance(extra_args, str):
+        elif isinstance(extra_args, str):
             extra_args = [extra_args]
         if dependencies is None:
             dependencies = []
         elif not isinstance(dependencies, list):
             dependencies = [dependencies]
-        # Add compile flags needed by dependencies after converting to the
+        # Add compile flags needed by dependencies
-        # native type of the selected compiler
+        args = CompilerArgs(self)
-        cargs = [a for d in dependencies for a in d.get_compile_args()]
+        for d in dependencies:
-        args = self.unix_link_flags_to_native(cargs)
+            args += d.get_compile_args()
         # Read c_args/cpp_args/etc from the cross-info file (if needed)
         args += self.get_cross_extra_flags(env, compile=True, link=False)
         # Add CFLAGS/CXXFLAGS/OBJCFLAGS/OBJCXXFLAGS from the env
         # We assume that the user has ensured these are compiler-specific
         args += env.coredata.external_args[self.language]
-        # Append extra_args to the compiler check args such that it overrides
+        args += self.get_compiler_check_args()
-        extra_args = self._override_args(self.get_compiler_check_args(), extra_args)
+        # extra_args must override all other arguments, so we add them last
-        extra_args = self.unix_link_flags_to_native(extra_args)
+        args += extra_args
-        # Append both to the compiler args such that they override them
-        args = self._override_args(args, extra_args)
         # We only want to compile; not link
-        with self.compile(code, args, mode) as p:
+        with self.compile(code, args.to_native(), mode) as p:
             return p.returncode == 0
 
     def _links_wrapper(self, code, env, extra_args, dependencies):
@@ -799,11 +945,11 @@ class CCompiler(Compiler):
             dependencies = []
         elif not isinstance(dependencies, list):
             dependencies = [dependencies]
-        # Add compile and link flags needed by dependencies after converting to
+        # Add compile and link flags needed by dependencies
-        # the native type of the selected compiler
+        args = CompilerArgs(self)
-        cargs = [a for d in dependencies for a in d.get_compile_args()]
+        for d in dependencies:
-        link_args = [a for d in dependencies for a in d.get_link_args()]
+            args += d.get_compile_args()
-        args = self.unix_link_flags_to_native(cargs + link_args)
+            args += d.get_link_args()
         # Select a CRT if needed since we're linking
         args += self.get_linker_debug_crt_args()
         # Read c_args/c_link_args/cpp_args/cpp_link_args/etc from the
@@ -812,12 +958,11 @@ class CCompiler(Compiler):
         # Add LDFLAGS from the env. We assume that the user has ensured these
         # are compiler-specific
         args += env.coredata.external_link_args[self.language]
-        # Append extra_args to the compiler check args such that it overrides
+        # Add compiler check args such that they override
-        extra_args = self._override_args(self.get_compiler_check_args(), extra_args)
+        args += self.get_compiler_check_args()
-        extra_args = self.unix_link_flags_to_native(extra_args)
+        # extra_args must override all other arguments, so we add them last
-        # Append both to the compiler args such that they override them
+        args += extra_args
-        args = self._override_args(args, extra_args)
+        return self.compile(code, args.to_native())
-        return self.compile(code, args)
 
     def links(self, code, env, extra_args=None, dependencies=None):
         with self._links_wrapper(code, env, extra_args, dependencies) as p:
@@ -1686,7 +1831,8 @@ class DCompiler(Compiler):
                 paths = paths + ':' + padding
         return ['-L-rpath={}'.format(paths)]
 
-    def translate_args_to_nongnu(self, args):
+    @classmethod
+    def translate_args_to_nongnu(cls, args):
         dcargs = []
         # Translate common arguments to flags the LDC/DMD compilers
         # can understand.
@@ -1802,11 +1948,9 @@ class LLVMDCompiler(DCompiler):
         # -L is for the compiler, telling it to pass the second -L to the linker.
         return ['-L-L' + dirname]
 
-    def unix_link_flags_to_native(self, args):
+    @classmethod
-        return self.translate_args_to_nongnu(args)
+    def unix_args_to_native(cls, args):
-
+        return cls.translate_args_to_nongnu(args)
-    def unix_compile_flags_to_native(self, args):
-        return self.translate_args_to_nongnu(args)
 
 class DmdDCompiler(DCompiler):
     def __init__(self, exelist, version, is_cross):
@@ -1854,11 +1998,9 @@ class DmdDCompiler(DCompiler):
     def get_std_shared_lib_link_args(self):
         return ['-shared', '-defaultlib=libphobos2.so']
 
-    def unix_link_flags_to_native(self, args):
+    @classmethod
-        return self.translate_args_to_nongnu(args)
+    def unix_args_to_native(cls, args):
-
+        return cls.translate_args_to_nongnu(args)
-    def unix_compile_flags_to_native(self, args):
-        return self.translate_args_to_nongnu(args)
 
 class VisualStudioCCompiler(CCompiler):
     std_warn_args = ['/W3']
@@ -1978,9 +2120,14 @@ class VisualStudioCCompiler(CCompiler):
     def get_option_link_args(self, options):
         return options['c_winlibs'].value[:]
 
-    def unix_link_flags_to_native(self, args):
+    @classmethod
+    def unix_args_to_native(cls, args):
         result = []
         for i in args:
+            # -mms-bitfields is specific to MinGW-GCC
+            # -pthread is only valid for GCC
+            if i in ('-mms-bitfields', '-pthread'):
+                continue
             if i.startswith('-L'):
                 i = '/LIBPATH:' + i[2:]
             # Translate GNU-style -lfoo library name to the import library
@@ -1998,16 +2145,6 @@ class VisualStudioCCompiler(CCompiler):
             result.append(i)
         return result
 
-    def unix_compile_flags_to_native(self, args):
-        result = []
-        for i in args:
-            # -mms-bitfields is specific to MinGW-GCC
-            # -pthread is only valid for GCC
-            if i in ('-mms-bitfields', '-pthread'):
-                continue
-            result.append(i)
-        return result
-
     def get_werror_args(self):
         return ['/WX']
 
@@ -2795,8 +2932,10 @@ class NAGFortranCompiler(FortranCompiler):
     def get_warn_args(self, level):
         return NAGFortranCompiler.std_warn_args
 
+class StaticLinker:
+    pass
 
-class VisualStudioLinker:
+class VisualStudioLinker(StaticLinker):
     always_args = ['/NOLOGO']
 
     def __init__(self, exelist):
@@ -2832,18 +2971,16 @@ class VisualStudioLinker:
     def get_option_link_args(self, options):
         return []
 
-    def unix_link_flags_to_native(self, args):
+    @classmethod
-        return args[:]
+    def unix_args_to_native(cls, args):
-
+        return VisualStudioCCompiler.unix_args_to_native(args)
-    def unix_compile_flags_to_native(self, args):
-        return args[:]
 
     def get_link_debugfile_args(self, targetfile):
         pdbarr = targetfile.split('.')[:-1]
         pdbarr += ['pdb']
         return ['/DEBUG', '/PDB:' + '.'.join(pdbarr)]
 
-class ArLinker:
+class ArLinker(StaticLinker):
 
     def __init__(self, exelist):
         self.exelist = exelist
@@ -2885,10 +3022,8 @@ class ArLinker:
     def get_option_link_args(self, options):
         return []
 
-    def unix_link_flags_to_native(self, args):
+    @classmethod
-        return args[:]
+    def unix_args_to_native(cls, args):
-
-    def unix_compile_flags_to_native(self, args):
         return args[:]
 
     def get_link_debugfile_args(self, targetfile):

run_unittests.py

@@ -19,6 +19,7 @@ import subprocess
 import re, json
 import tempfile
 from glob import glob
+import mesonbuild.compilers
 import mesonbuild.environment
 import mesonbuild.mesonlib
 from mesonbuild.environment import detect_ninja, Environment
@@ -95,6 +96,62 @@ class InternalTests(unittest.TestCase):
                          stat.S_IRWXU | stat.S_ISUID |
                          stat.S_IRGRP | stat.S_IXGRP)
 
+    def test_compiler_args_class(self):
+        cargsfunc = mesonbuild.compilers.CompilerArgs
+        c = mesonbuild.environment.CCompiler([], 'fake', False)
+        # Test that bad initialization fails
+        self.assertRaises(TypeError, cargsfunc, [])
+        self.assertRaises(TypeError, cargsfunc, [], [])
+        self.assertRaises(TypeError, cargsfunc, c, [], [])
+        # Test that empty initialization works
+        a = cargsfunc(c)
+        self.assertEqual(a, [])
+        # Test that list initialization works
+        a = cargsfunc(['-I.', '-I..'], c)
+        self.assertEqual(a, ['-I.', '-I..'])
+        # Test that there is no de-dup on initialization
+        self.assertEqual(cargsfunc(['-I.', '-I.'], c), ['-I.', '-I.'])
+
+        ## Test that appending works
+        a.append('-I..')
+        self.assertEqual(a, ['-I..', '-I.'])
+        a.append('-O3')
+        self.assertEqual(a, ['-I..', '-I.', '-O3'])
+
+        ## Test that in-place addition works
+        a += ['-O2', '-O2']
+        self.assertEqual(a, ['-I..', '-I.', '-O3', '-O2', '-O2'])
+        # Test that removal works
+        a.remove('-O2')
+        self.assertEqual(a, ['-I..', '-I.', '-O3', '-O2'])
+        # Test that de-dup happens on addition
+        a += ['-Ifoo', '-Ifoo']
+        self.assertEqual(a, ['-Ifoo', '-I..', '-I.', '-O3', '-O2'])
+
+        # .extend() is just +=, so we don't test it
+
+        ## Test that addition works
+        # Test that adding a list with just one old arg works and yields the same array
+        a = a + ['-Ifoo']
+        self.assertEqual(a, ['-Ifoo', '-I..', '-I.', '-O3', '-O2'])
+        # Test that adding a list with one arg new and one old works
+        a = a + ['-Ifoo', '-Ibaz']
+        self.assertEqual(a, ['-Ifoo', '-Ibaz', '-I..', '-I.', '-O3', '-O2'])
+        # Test that adding args that must be prepended and appended works
+        a = a + ['-Ibar', '-Wall']
+        self.assertEqual(a, ['-Ibar', '-Ifoo', '-Ibaz', '-I..', '-I.', '-O3', '-O2', '-Wall'])
+
+        ## Test that reflected addition works
+        # Test that adding to a list with just one old arg works and DOES NOT yield the same array
+        a = ['-Ifoo'] + a
+        self.assertEqual(a, ['-Ibar', '-Ifoo', '-Ibaz', '-I..', '-I.', '-O3', '-O2', '-Wall'])
+        # Test that adding to a list with just one new arg that is not pre-pended works
+        a = ['-Werror'] + a
+        self.assertEqual(a, ['-Ibar', '-Ifoo', '-Ibaz', '-I..', '-I.', '-Werror', '-O3', '-O2', '-Wall'])
+        # Test that adding to a list with two new args preserves the order
+        a = ['-Ldir', '-Lbah'] + a
+        self.assertEqual(a, ['-Ibar', '-Ifoo', '-Ibaz', '-I..', '-I.', '-Ldir', '-Lbah', '-Werror', '-O3', '-O2', '-Wall'])
+
 
 class LinuxlikeTests(unittest.TestCase):
     def setUp(self):
@@ -416,7 +473,7 @@ class LinuxlikeTests(unittest.TestCase):
                 cmd = cmd[1:]
             # Verify that -I flags from the `args` kwarg are first
             # This is set in the '43 has function' test case
-            self.assertEqual(cmd[2], '-I/tmp')
+            self.assertEqual(cmd[1], '-I/tmp')
             # Verify that -O3 set via the environment is overriden by -O0
             Oargs = [arg for arg in cmd if arg.startswith('-O')]
             self.assertEqual(Oargs, [Oflag, '-O0'])