Use CompilerArgs for generation of compile commands

At the same time, also fix the order in which compile arguments are
added. Detailed comments have been added concerning the priority and
order of the arguments.

Also adds a unit test and an integration test for the same.

18 files changed, 238 insertions, 65 deletions

diff --git a/mesonbuild/backend/backends.py b/mesonbuild/backend/backends.py
index eadc8cc..a83d95f 100644
--- a/mesonbuild/backend/backends.py
+++ b/mesonbuild/backend/backends.py
@@ -21,6 +21,7 @@ from .. import compilers
 import json
 import subprocess
 from ..mesonlib import MesonException, get_compiler_for_source, classify_unity_sources
+from ..compilers import CompilerArgs
 
 class CleanTrees:
     '''
@@ -338,32 +339,59 @@ class Backend:
         return extra_args
 
     def generate_basic_compiler_args(self, target, compiler, no_warn_args=False):
-        commands = []
+        # Create an empty commands list, and start adding arguments from
+        # various sources in the order in which they must override each other
+        # starting from hard-coded defaults followed by build options and so on.
+        commands = CompilerArgs(compiler)
+        # First, the trivial ones that are impossible to override.
+        #
+        # Add -nostdinc/-nostdinc++ if needed; can't be overriden
         commands += self.get_cross_stdlib_args(target, compiler)
+        # Add things like /NOLOGO or -pipe; usually can't be overriden
         commands += compiler.get_always_args()
+        # Only add warning-flags by default if the buildtype enables it, and if
+        # we weren't explicitly asked to not emit warnings (for Vala, f.ex)
         if no_warn_args:
             commands += compiler.get_no_warn_args()
         elif self.environment.coredata.get_builtin_option('buildtype') != 'plain':
             commands += compiler.get_warn_args(self.environment.coredata.get_builtin_option('warning_level'))
+        # Add -Werror if werror=true is set in the build options set on the
+        # command-line or default_options inside project(). This only sets the
+        # action to be done for warnings if/when they are emitted, so it's ok
+        # to set it after get_no_warn_args() or get_warn_args().
+        if self.environment.coredata.get_builtin_option('werror'):
+            commands += compiler.get_werror_args()
+        # Add compile args for c_* or cpp_* build options set on the
+        # command-line or default_options inside project().
         commands += compiler.get_option_compile_args(self.environment.coredata.compiler_options)
-        commands += self.build.get_global_args(compiler)
+        # Add buildtype args: optimization level, debugging, etc.
+        commands += compiler.get_buildtype_args(self.environment.coredata.get_builtin_option('buildtype'))
+        # Add compile args added using add_project_arguments()
         commands += self.build.get_project_args(compiler, target.subproject)
+        # Add compile args added using add_global_arguments()
+        # These override per-project arguments
+        commands += self.build.get_global_args(compiler)
+        # Compile args added from the env: CFLAGS/CXXFLAGS, etc. We want these
+        # to override all the defaults, but not the per-target compile args.
         commands += self.environment.coredata.external_args[compiler.get_language()]
-        commands += self.escape_extra_args(compiler, target.get_extra_args(compiler.get_language()))
-        commands += compiler.get_buildtype_args(self.environment.coredata.get_builtin_option('buildtype'))
-        if self.environment.coredata.get_builtin_option('werror'):
-            commands += compiler.get_werror_args()
+        # Always set -fPIC for shared libraries
         if isinstance(target, build.SharedLibrary):
             commands += compiler.get_pic_args()
+        # Set -fPIC for static libraries by default unless explicitly disabled
         if isinstance(target, build.StaticLibrary) and target.pic:
             commands += compiler.get_pic_args()
+        # Add compile args needed to find external dependencies
+        # Link args are added while generating the link command
         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_args_to_native(dep.get_compile_args())
+            commands += dep.get_compile_args()
+            # Qt needs -fPIC for executables
+            # XXX: We should move to -fPIC for all executables
             if isinstance(target, build.Executable):
-                commands += dep.get_exe_args()
-
+                commands += dep.get_exe_args(compiler)
+            # For 'automagic' deps: Boost and GTest. Also dependency('threads').
+            # pkg-config puts the thread flags itself via `Cflags:`
+            if dep.need_threads():
+                commands += compiler.thread_flags()
         # Fortran requires extra include directives.
         if compiler.language == 'fortran':
             for lt in target.link_targets:
diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py
index 5bd660c..98740a4 100644
--- a/mesonbuild/backend/ninjabackend.py
+++ b/mesonbuild/backend/ninjabackend.py
@@ -19,6 +19,7 @@ from .. import build
 from .. import mlog
 from .. import dependencies
 from .. import compilers
+from ..compilers import CompilerArgs
 from ..mesonlib import File, MesonException, get_compiler_for_source, Popen_safe
 from .backends import CleanTrees, InstallData
 from ..build import InvalidArguments
@@ -1725,7 +1726,7 @@ rule FORTRAN_DEP_HACK
 
     def generate_llvm_ir_compile(self, target, outfile, src):
         compiler = get_compiler_for_source(target.compilers.values(), src)
-        commands = []
+        commands = CompilerArgs(compiler)
         # Compiler args for compiling this target
         commands += compilers.get_base_compile_args(self.environment.coredata.base_options,
                                                     compiler)
@@ -1748,11 +1749,40 @@ rule FORTRAN_DEP_HACK
         # Write the Ninja build command
         compiler_name = 'llvm_ir{}_COMPILER'.format('_CROSS' if target.is_cross else '')
         element = NinjaBuildElement(self.all_outputs, rel_obj, compiler_name, rel_src)
-        commands = self.dedup_arguments(commands)
+        # Convert from GCC-style link argument naming to the naming used by the
+        # current compiler.
+        commands = commands.to_native()
         element.add_item('ARGS', commands)
         element.write(outfile)
         return rel_obj
 
+    def get_source_dir_include_args(self, target, compiler):
+        curdir = target.get_subdir()
+        tmppath = os.path.normpath(os.path.join(self.build_to_src, curdir))
+        return compiler.get_include_args(tmppath, False)
+
+    def get_build_dir_include_args(self, target, compiler):
+        curdir = target.get_subdir()
+        if curdir == '':
+            curdir = '.'
+        return compiler.get_include_args(curdir, False)
+
+    def get_custom_target_dir_include_args(self, target, compiler):
+        custom_target_include_dirs = []
+        for i in target.get_generated_sources():
+            # Generator output goes into the target private dir which is
+            # already in the include paths list. Only custom targets have their
+            # own target build dir.
+            if not isinstance(i, build.CustomTarget):
+                continue
+            idir = self.get_target_dir(i)
+            if idir not in custom_target_include_dirs:
+                custom_target_include_dirs.append(idir)
+        incs = []
+        for i in custom_target_include_dirs:
+            incs += compiler.get_include_args(i, False)
+        return incs
+
     def generate_single_compile(self, target, outfile, src, is_generated=False, header_deps=[], order_deps=[]):
         """
         Compiles C/C++, ObjC/ObjC++, Fortran, and D sources
@@ -1763,30 +1793,40 @@ rule FORTRAN_DEP_HACK
             raise AssertionError('BUG: sources should not contain headers {!r}'.format(src.fname))
         extra_orderdeps = []
         compiler = get_compiler_for_source(target.compilers.values(), src)
-        commands = []
-        # The first thing is implicit include directories: source, build and private.
-        commands += compiler.get_include_args(self.get_target_private_dir(target), False)
-        # Compiler args for compiling this target
+
+        # Create an empty commands list, and start adding arguments from
+        # various sources in the order in which they must override each other
+        commands = CompilerArgs(compiler)
+        # Add compiler args for compiling this target derived from 'base' build
+        # options passed on the command-line, in default_options, etc.
+        # These have the lowest priority.
         commands += compilers.get_base_compile_args(self.environment.coredata.base_options,
                                                     compiler)
-        # Add the root source and build directories as include dirs
-        curdir = target.get_subdir()
-        tmppath = os.path.normpath(os.path.join(self.build_to_src, curdir))
-        src_inc = compiler.get_include_args(tmppath, False)
-        if curdir == '':
-            curdir = '.'
-        build_inc = compiler.get_include_args(curdir, False)
-        commands += build_inc + src_inc
-        # -I args work differently than other ones. In them the first found
-        # directory is used whereas for other flags (such as -ffoo -fno-foo) the
-        # latest one is used.  Therefore put the internal include directories
-        # here before generating the "basic compiler args" so they override args
-        # coming from e.g. pkg-config.
+        # The code generated by valac is usually crap and has tons of unused
+        # variables and such, so disable warnings for Vala C sources.
+        no_warn_args = (is_generated == 'vala')
+        # Add compiler args and include paths from several sources; defaults,
+        # build options, external dependencies, etc.
+        commands += self.generate_basic_compiler_args(target, compiler, no_warn_args)
+        # Add include dirs from the `include_directories:` kwarg on the target
+        # and from `include_directories:` of internal deps of the target.
+        #
+        # Target include dirs should override internal deps include dirs.
+        #
+        # Include dirs from internal deps should override include dirs from
+        # external deps.
         for i in target.get_include_dirs():
             basedir = i.get_curdir()
             for d in i.get_incdirs():
-                expdir = os.path.join(basedir, d)
+                # Avoid superfluous '/.' at the end of paths when d is '.'
+                if d not in ('', '.'):
+                    expdir = os.path.join(basedir, d)
+                else:
+                    expdir = basedir
                 srctreedir = os.path.join(self.build_to_src, expdir)
+                # Add source subdir first so that the build subdir overrides it
+                sargs = compiler.get_include_args(srctreedir, i.is_system)
+                commands += sargs
                 # There may be include dirs where a build directory has not been
                 # created for some source dir. For example if someone does this:
                 #
@@ -1797,20 +1837,32 @@ rule FORTRAN_DEP_HACK
                     bargs = compiler.get_include_args(expdir, i.is_system)
                 else:
                     bargs = []
-                sargs = compiler.get_include_args(srctreedir, i.is_system)
                 commands += bargs
-                commands += sargs
             for d in i.get_extra_build_dirs():
                 commands += compiler.get_include_args(d, i.is_system)
-        commands += self.generate_basic_compiler_args(target, compiler,
-                                                      # The code generated by valac is usually crap
-                                                      # and has tons of unused variables and such,
-                                                      # so disable warnings for Vala C sources.
-                                                      no_warn_args=(is_generated == 'vala'))
-        for d in target.external_deps:
-            if d.need_threads():
-                commands += compiler.thread_flags()
-                break
+        # Add per-target compile args, f.ex, `c_args : ['-DFOO']`. We set these
+        # near the end since these are supposed to override everything else.
+        commands += self.escape_extra_args(compiler,
+                                           target.get_extra_args(compiler.get_language()))
+        # Add source dir and build dir. Project-specific and target-specific
+        # include paths must override per-target compile args, include paths
+        # from external dependencies, internal dependencies, and from
+        # per-target `include_directories:`
+        #
+        # We prefer headers in the build dir and the custom target dir over the
+        # source dir since, for instance, the user might have an
+        # srcdir == builddir Autotools build in their source tree. Many
+        # projects that are moving to Meson have both Meson and Autotools in
+        # parallel as part of the transition.
+        commands += self.get_source_dir_include_args(target, compiler)
+        commands += self.get_custom_target_dir_include_args(target, compiler)
+        commands += self.get_build_dir_include_args(target, compiler)
+        # Finally add the private dir for the target to the include path. This
+        # must override everything else and must be the final path added.
+        commands += compiler.get_include_args(self.get_target_private_dir(target), False)
+
+        # FIXME: This file handling is atrocious and broken. We need to
+        # replace it with File objects used consistently everywhere.
         if isinstance(src, RawFilename):
             rel_src = src.fname
             if os.path.isabs(src.fname):
@@ -1835,7 +1887,13 @@ rule FORTRAN_DEP_HACK
         rel_obj = os.path.join(self.get_target_private_dir(target), obj_basename)
         rel_obj += '.' + self.environment.get_object_suffix()
         dep_file = compiler.depfile_for_object(rel_obj)
+
+        # Add MSVC debug file generation compile flags: /Fd /FS
+        commands += self.get_compile_debugfile_args(compiler, target, rel_obj)
+
+        # PCH handling
         if self.environment.coredata.base_options.get('b_pch', False):
+            commands += self.get_pch_include_args(compiler, target)
             pchlist = target.get_pch(compiler.language)
         else:
             pchlist = []
@@ -1848,19 +1906,7 @@ rule FORTRAN_DEP_HACK
             i = os.path.join(self.get_target_private_dir(target), compiler.get_pch_name(pchlist[0]))
             arr.append(i)
             pch_dep = arr
-        custom_target_include_dirs = []
-        for i in target.get_generated_sources():
-            if not isinstance(i, build.CustomTarget):
-                continue
-            idir = self.get_target_dir(i)
-            if idir not in custom_target_include_dirs:
-                custom_target_include_dirs.append(idir)
-        for i in custom_target_include_dirs:
-            commands += compiler.get_include_args(i, False)
-        if self.environment.coredata.base_options.get('b_pch', False):
-            commands += self.get_pch_include_args(compiler, target)
 
-        commands += self.get_compile_debugfile_args(compiler, target, rel_obj)
         crstr = ''
         if target.is_cross:
             crstr = '_CROSS'
@@ -1895,7 +1941,9 @@ rule FORTRAN_DEP_HACK
             element.add_orderdep(d)
         element.add_orderdep(pch_dep)
         element.add_orderdep(extra_orderdeps)
-        commands = self.dedup_arguments(commands)
+        # Convert from GCC-style link argument naming to the naming used by the
+        # current compiler.
+        commands = commands.to_native()
         for i in self.get_fortran_orderdeps(target, compiler):
             element.add_orderdep(i)
         element.add_item('DEPFILE', dep_file)
diff --git a/mesonbuild/build.py b/mesonbuild/build.py
index a9f08c3..e5d2284 100644
--- a/mesonbuild/build.py
+++ b/mesonbuild/build.py
@@ -585,14 +585,16 @@ class BuildTarget(Target):
         for i in self.link_depends:
             if not isinstance(i, str):
                 raise InvalidArguments('Link_depends arguments must be strings.')
-        inclist = kwargs.get('include_directories', [])
-        if not isinstance(inclist, list):
-            inclist = [inclist]
-        self.add_include_dirs(inclist)
         deplist = kwargs.get('dependencies', [])
         if not isinstance(deplist, list):
             deplist = [deplist]
         self.add_deps(deplist)
+        # Target-specific include dirs must be added after include dirs from
+        # internal deps (added inside self.add_deps()) to override correctly.
+        inclist = kwargs.get('include_directories', [])
+        if not isinstance(inclist, list):
+            inclist = [inclist]
+        self.add_include_dirs(inclist)
         self.custom_install_dir = kwargs.get('install_dir', None)
         if self.custom_install_dir is not None:
             if not isinstance(self.custom_install_dir, str):
diff --git a/mesonbuild/coredata.py b/mesonbuild/coredata.py
index 60bb10b..1ec769a 100644
--- a/mesonbuild/coredata.py
+++ b/mesonbuild/coredata.py
@@ -117,7 +117,9 @@ class CoreData:
         self.user_options = {}
         self.compiler_options = {}
         self.base_options = {}
-        self.external_args = {} # These are set from "the outside" with e.g. mesonconf
+        # These two, external_*args, are set via env vars CFLAGS, LDFLAGS, etc
+        # but only when not cross-compiling.
+        self.external_args = {}
         self.external_link_args = {}
         if options.cross_file is not None:
             self.cross_file = os.path.join(os.getcwd(), options.cross_file)
diff --git a/mesonbuild/dependencies.py b/mesonbuild/dependencies.py
index 6ae91d4..b01e0a8 100644
--- a/mesonbuild/dependencies.py
+++ b/mesonbuild/dependencies.py
@@ -59,7 +59,7 @@ class Dependency:
     def get_name(self):
         return self.name
 
-    def get_exe_args(self):
+    def get_exe_args(self, compiler):
         return []
 
     def need_threads(self):
@@ -1045,16 +1045,14 @@ class QtBaseDependency(Dependency):
     def found(self):
         return self.is_found
 
-    def get_exe_args(self):
+    def get_exe_args(self, compiler):
         # Originally this was -fPIE but nowadays the default
         # for upstream and distros seems to be -reduce-relocations
         # which requires -fPIC. This may cause a performance
         # penalty when using self-built Qt or on platforms
         # where -fPIC is not required. If this is an issue
         # for you, patches are welcome.
-        if mesonlib.is_linux():
-            return ['-fPIC']
-        return []
+        return compiler.get_pic_args()
 
 class Qt5Dependency(QtBaseDependency):
     def __init__(self, env, kwargs):
diff --git a/run_unittests.py b/run_unittests.py
index a90e46b..b6ea073 100755
--- a/run_unittests.py
+++ b/run_unittests.py
@@ -14,6 +14,7 @@
 # limitations under the License.
 
 import stat
+import shlex
 import unittest, os, sys, shutil, time
 import subprocess
 import re, json
@@ -698,6 +699,38 @@ class LinuxlikeTests(unittest.TestCase):
             # The chown failed nonfatally if we're not root
             self.assertEqual(0, statf.st_uid)
 
+    def test_internal_include_order(self):
+        testdir = os.path.join(self.common_test_dir, '138 include order')
+        self.init(testdir)
+        for cmd in self.get_compdb():
+            if cmd['file'].endswith('/main.c'):
+                cmd = cmd['command']
+                break
+        else:
+            raise Exception('Could not find main.c command')
+        incs = [a for a in shlex.split(cmd) if a.startswith("-I")]
+        self.assertEqual(len(incs), 8)
+        # target private dir
+        self.assertEqual(incs[0], "-Isub4/someexe@exe")
+        # target build subdir
+        self.assertEqual(incs[1], "-Isub4")
+        # target source subdir
+        msg = "{!r} does not end with '/sub4'".format(incs[2])
+        self.assertTrue(incs[2].endswith("/sub4"), msg)
+        # include paths added via per-target c_args: ['-I'...]
+        msg = "{!r} does not end with '/sub3'".format(incs[3])
+        self.assertTrue(incs[3].endswith("/sub3"), msg)
+        # target include_directories: build dir
+        self.assertEqual(incs[4], "-Isub2")
+        # target include_directories: source dir
+        msg = "{!r} does not end with '/sub2'".format(incs[5])
+        self.assertTrue(incs[5].endswith("/sub2"), msg)
+        # target internal dependency include_directories: build dir
+        self.assertEqual(incs[6], "-Isub1")
+        # target internal dependency include_directories: source dir
+        msg = "{!r} does not end with '/sub1'".format(incs[7])
+        self.assertTrue(incs[7].endswith("/sub1"), msg)
+
 
 class RewriterTests(unittest.TestCase):
 
diff --git a/test cases/common/138 include order/meson.build b/test cases/common/138 include order/meson.build
new file mode 100644
index 0000000..f744ae7
--- /dev/null
+++ b/test cases/common/138 include order/meson.build
@@ -0,0 +1,22 @@
+project('include order', 'c')
+
+# Test that the order of priority of include paths (from first to last) is:
+#
+# 1. Target's current build directory
+# 2. Target's current source directory
+# 3. Include paths added with the `c_args:` kwarg
+# 4. Include paths added with the `include_directories`: kwarg
+#    Within this, the build dir takes precedence over the source dir
+# 5. Include paths added via `include_directories:` of internal deps
+#    Within this, the build dir takes precedence over the source dir
+
+# Defines an internal dep
+subdir('sub1')
+# Defines a per-target include path
+subdir('sub2')
+# Directory for `c_args:` include path
+subdir('sub3')
+# The directory where the target resides
+subdir('sub4')
+
+test('eh', e)
diff --git a/test cases/common/138 include order/sub1/main.h b/test cases/common/138 include order/sub1/main.h
new file mode 100644
index 0000000..acf4a35
--- /dev/null
+++ b/test cases/common/138 include order/sub1/main.h
@@ -0,0 +1 @@
+#error "sub1/main.h included"
diff --git a/test cases/common/138 include order/sub1/meson.build b/test cases/common/138 include order/sub1/meson.build
new file mode 100644
index 0000000..9672945
--- /dev/null
+++ b/test cases/common/138 include order/sub1/meson.build
@@ -0,0 +1,4 @@
+i = include_directories('.')
+l = shared_library('somelib', 'some.c')
+dep = declare_dependency(link_with : l,
+                         include_directories : i)
diff --git a/test cases/common/138 include order/sub1/some.c b/test cases/common/138 include order/sub1/some.c
new file mode 100644
index 0000000..1ab0db4
--- /dev/null
+++ b/test cases/common/138 include order/sub1/some.c
@@ -0,0 +1,6 @@
+#if defined _WIN32 || defined __CYGWIN__
+  __declspec(dllexport)
+#endif
+int somefunc(void) {
+  return 1984;
+}
diff --git a/test cases/common/138 include order/sub1/some.h b/test cases/common/138 include order/sub1/some.h
new file mode 100644
index 0000000..6479492
--- /dev/null
+++ b/test cases/common/138 include order/sub1/some.h
@@ -0,0 +1,10 @@
+#pragma once
+
+#if defined _WIN32 || defined __CYGWIN__
+  #define DLL_PUBLIC __declspec(dllimport)
+#else
+  #define DLL_PUBLIC
+#endif
+
+DLL_PUBLIC
+int somefunc(void);
diff --git a/test cases/common/138 include order/sub2/main.h b/test cases/common/138 include order/sub2/main.h
new file mode 100644
index 0000000..b9c0da9
--- /dev/null
+++ b/test cases/common/138 include order/sub2/main.h
@@ -0,0 +1 @@
+#error "sub2/main.h included"
diff --git a/test cases/common/138 include order/sub2/meson.build b/test cases/common/138 include order/sub2/meson.build
new file mode 100644
index 0000000..7b49d6a
--- /dev/null
+++ b/test cases/common/138 include order/sub2/meson.build
@@ -0,0 +1 @@
+j = include_directories('.')
diff --git a/test cases/common/138 include order/sub3/main.h b/test cases/common/138 include order/sub3/main.h
new file mode 100644
index 0000000..1ab7231
--- /dev/null
+++ b/test cases/common/138 include order/sub3/main.h
@@ -0,0 +1 @@
+#error "sub3/main.h included"
diff --git a/test cases/common/138 include order/sub3/meson.build b/test cases/common/138 include order/sub3/meson.build
new file mode 100644
index 0000000..0bd3906
--- /dev/null
+++ b/test cases/common/138 include order/sub3/meson.build
@@ -0,0 +1 @@
+sub3 = meson.current_source_dir()
diff --git a/test cases/common/138 include order/sub4/main.c b/test cases/common/138 include order/sub4/main.c
new file mode 100644
index 0000000..0b25eed
--- /dev/null
+++ b/test cases/common/138 include order/sub4/main.c
@@ -0,0 +1,8 @@
+/* Use the <> include notation to force searching in include directories */
+#include <main.h>
+
+int main(int argc, char *argv[]) {
+  if (somefunc() == 1984)
+    return 0;
+  return 1;
+}
diff --git a/test cases/common/138 include order/sub4/main.h b/test cases/common/138 include order/sub4/main.h
new file mode 100644
index 0000000..194d7fe
--- /dev/null
+++ b/test cases/common/138 include order/sub4/main.h
@@ -0,0 +1,3 @@
+#pragma once
+
+#include "some.h"
diff --git a/test cases/common/138 include order/sub4/meson.build b/test cases/common/138 include order/sub4/meson.build
new file mode 100644
index 0000000..538899a
--- /dev/null
+++ b/test cases/common/138 include order/sub4/meson.build
@@ -0,0 +1,4 @@
+e = executable('someexe', 'main.c',
+               c_args : ['-I' + sub3],
+               include_directories : j,
+               dependencies : dep)