interpreterbase: warn on redundant Meson version checks

We might encounter a Meson version check that always evaluates to true
or to false:

    project('t', 'c', meson_version: '>=0.60.0')
    if meson.version().version_compare('>=0.55.0')
      v = 1
    endif
    if meson.version().version_compare('<0.60.0')
      v = 2
    endif

Print warnings in such cases.

Only the innermost version dependency is available to us, so we don't
catch cases like:

    project('t', 'c', meson_version: '>=0.50')
    if meson.version().version_compare('<0.60')
      v = 1
      if meson.version().version_compare('>=0.40')
        v = 2
      endif
    endif

but such constructs aren't typical.

Author: bgilbert

mesonbuild/interpreterbase/interpreterbase.py

@@ -309,15 +309,20 @@ def evaluate_if(self, node: mparser.IfClauseNode) -> T.Optional[Disabler]:
             res = result.operator_call(MesonOperator.BOOL, None)
             if not isinstance(res, bool):
                 raise InvalidCode(f'If clause {result!r} does not evaluate to true or false.')
-            if res:
-                prev_meson_version = mesonlib.project_meson_versions[self.subproject]
-                if self.tmp_meson_version:
-                    mesonlib.project_meson_versions[self.subproject] = self.tmp_meson_version
-                try:
+            prev_meson_version = mesonlib.project_meson_versions[self.subproject]
+            if self.tmp_meson_version:
+                always = mesonlib.version_compare_conditions(prev_meson_version,
+                                                             self.tmp_meson_version)
+                if always is not None:
+                    mlog.warning(f"Version comparison '{self.tmp_meson_version}' always evaluates to {str(always).lower()}",
+                                 location=self.current_node)
+                mesonlib.project_meson_versions[self.subproject] = self.tmp_meson_version
+            try:
+                if res:
                     self.evaluate_codeblock(i.block)
-                finally:
-                    mesonlib.project_meson_versions[self.subproject] = prev_meson_version
-                return None
+                    return None
+            finally:
+                mesonlib.project_meson_versions[self.subproject] = prev_meson_version
         if not isinstance(node.elseblock, mparser.EmptyNode):
             self.evaluate_codeblock(node.elseblock.block)
         return None

mesonbuild/utils/universal.py

@@ -160,6 +160,7 @@ class _VerPickleLoadable(Protocol):
     'unique_list',
     'verbose_git',
     'version_compare',
+    'version_compare_conditions',
     'version_compare_condition_with_min',
     'version_compare_many',
     'search_version',
@@ -991,6 +992,52 @@ def version_compare_condition_with_min(condition: str, minimum: str) -> bool:
 
     return T.cast('bool', cmpop(Version(minimum), Version(condition)))
 
+
+# given the Meson version condition |outer_cond|, determine if the version
+# condition |inner_cond| is always true or always false
+def version_compare_conditions(outer_cond: str, inner_cond: str) -> T.Optional[bool]:
+    class Extracted:
+        def __init__(self, v: str):
+            self.op, val = _version_extract_cmpop(v)
+            val += '.0' * max(0, 2 - val.count('.'))
+            self.val = Version(val)
+            if self.op == operator.ne:
+                self.direction: T.Callable[[T.Any, T.Any], bool] = operator.eq
+            elif self.op == operator.le:
+                self.direction = operator.lt
+            elif self.op == operator.ge:
+                self.direction = operator.gt
+            else:
+                self.direction = self.op
+            self.inclusive = self.op in (operator.eq, operator.le, operator.ge)
+
+    inner, outer = Extracted(inner_cond), Extracted(outer_cond)
+    if inner.val == outer.val:
+        if outer.op == inner.op:
+            return True
+        if outer.direction == operator.eq:
+            if outer.inclusive and inner.inclusive:
+                return True
+            if outer.inclusive and not inner.inclusive:
+                return False
+            if not outer.inclusive and inner.op == operator.eq:
+                return False
+        elif outer.inclusive:
+            if not inner.inclusive and inner.direction not in (operator.eq, outer.direction):
+                return False
+        elif outer.direction != inner.direction:
+            return inner.op == operator.ne
+        else:
+            return True
+    if inner.val < outer.val:
+        if outer.op == operator.eq or outer.direction == operator.gt:
+            return inner.direction == operator.gt or inner.op == operator.ne
+    if inner.val > outer.val:
+        if outer.op == operator.eq or outer.direction == operator.lt:
+            return inner.direction == operator.lt or inner.op == operator.ne
+    return None
+
+
 def search_version(text: str) -> str:
     # Usually of the type 4.1.4 but compiler output may contain
     # stuff like this:

test cases/common/286 redundant version check/meson.build

@@ -0,0 +1,7 @@
+project('t', 'c', meson_version: '>=0.60.0')
+if meson.version().version_compare('>=0.55.0')
+  v = 1
+endif
+if meson.version().version_compare('<0.60.0')
+  v = 2
+endif

test cases/common/286 redundant version check/test.json

@@ -0,0 +1,10 @@
+{
+  "stdout": [
+    {
+      "line": "test cases/common/286 redundant version check/meson.build:2: WARNING: Version comparison '>=0.55.0' always evaluates to true"
+    },
+    {
+      "line": "test cases/common/286 redundant version check/meson.build:5: WARNING: Version comparison '<0.60.0' always evaluates to false"
+    }
+  ]
+}

unittests/internaltests.py

@@ -7,6 +7,7 @@
 import argparse
 import contextlib
 import io
+import itertools
 import json
 import operator
 import os
@@ -830,6 +831,82 @@ def test_version_compare(self):
                 for o, name in [(operator.lt, 'lt'), (operator.le, 'le'), (operator.eq, 'eq')]:
                     self.assertFalse(o(ver_a, ver_b), f'{ver_a} {name} {ver_b}')
 
+    def test_version_compare_conditions(self):
+        # {outer_op: {inner_op: [inner < outer, inner == outer, inner > outer]}}
+        tests = {
+            '>=': {
+                '>=': [True, True, None],
+                '>': [True, None, None],
+                '<': [False, False, None],
+                '<=': [False, None, None],
+                '=': [False, None, None],
+                '==': [False, None, None],
+                '!=': [True, None, None],
+            },
+            '>': {
+                '>=': [True, True, None],
+                '>': [True, True, None],
+                '<': [False, False, None],
+                '<=': [False, False, None],
+                '=': [False, False, None],
+                '==': [False, False, None],
+                '!=': [True, True, None],
+            },
+            '<': {
+                '>=': [None, False, False],
+                '>': [None, False, False],
+                '<': [None, True, True],
+                '<=': [None, True, True],
+                '=': [None, False, False],
+                '==': [None, False, False],
+                '!=': [None, True, True],
+            },
+            '<=': {
+                '>=': [None, None, False],
+                '>': [None, False, False],
+                '<': [None, None, True],
+                '<=': [None, True, True],
+                '=': [None, None, False],
+                '==': [None, None, False],
+                '!=': [None, None, True],
+            },
+            '=': {
+                '>=': [True, True, False],
+                '>': [True, False, False],
+                '<': [False, False, True],
+                '<=': [False, True, True],
+                '=': [False, True, False],
+                '==': [False, True, False],
+                '!=': [True, False, True],
+            },
+            '==': {
+                '>=': [True, True, False],
+                '>': [True, False, False],
+                '<': [False, False, True],
+                '<=': [False, True, True],
+                '=': [False, True, False],
+                '==': [False, True, False],
+                '!=': [True, False, True],
+            },
+            '!=': {
+                '>=': [None, None, None],
+                '>': [None, None, None],
+                '<': [None, None, None],
+                '<=': [None, None, None],
+                '=': [None, False, None],
+                '==': [None, False, None],
+                '!=': [None, True, None],
+            },
+        }
+        sufs = ('', '.0')
+        for outer_op, inner in tests.items():
+            for inner_op, results in inner.items():
+                for inner_val, result in zip((40, 50, 60), results):
+                    for outer_ext, inner_ext in itertools.product(sufs, sufs):
+                        self.assertEqual(mesonbuild.mesonlib.version_compare_conditions(f'{outer_op}0.50{outer_ext}',
+                                                                                        f'{inner_op}0.{inner_val}{inner_ext}'),
+                                         result)
+
     def test_msvc_toolset_version(self):
         '''
         Ensure that the toolset version returns the correct value for this MSVC