Skip to content

Commit

Permalink
compilers: Use a dataclass instead of tuple for cached results
Browse files Browse the repository at this point in the history
There's been a number of bugs where the code does something like:
```python
def foo(...) -> T.Tuple[bool, bool]: ...

if foo():
   ...
```

Which will always resolve to True since a non-empty Tuple is truthy. To
avoid that I've moved to a simple struct that implements a bool dunder,
so that doing `if result:` does the expected thing.
  • Loading branch information
dcbaker committed Sep 13, 2024
1 parent 7813354 commit d31635c
Show file tree
Hide file tree
Showing 18 changed files with 199 additions and 184 deletions.
7 changes: 4 additions & 3 deletions mesonbuild/compilers/c.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
gnu_winlibs,
msvc_winlibs,
Compiler,
CompileCheckResult,
)

if T.TYPE_CHECKING:
Expand Down Expand Up @@ -79,7 +80,7 @@ def sanity_check(self, work_dir: str, environment: 'Environment') -> None:
def has_header_symbol(self, hname: str, symbol: str, prefix: str,
env: 'Environment', *,
extra_args: T.Union[None, T.List[str], T.Callable[['CompileCheckMode'], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
fargs = {'prefix': prefix, 'header': hname, 'symbol': symbol}
t = '''{prefix}
#include <{header}>
Expand Down Expand Up @@ -394,9 +395,9 @@ def get_options(self) -> 'MutableKeyedOptionDictType':
# So we should explicitly fail at this case.
def has_function(self, funcname: str, prefix: str, env: 'Environment', *,
extra_args: T.Optional[T.List[str]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
if funcname == 'lchmod':
return False, False
return CompileCheckResult(False)
else:
return super().has_function(funcname, prefix, env,
extra_args=extra_args,
Expand Down
74 changes: 42 additions & 32 deletions mesonbuild/compilers/compilers.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,19 @@ class CompileCheckMode(enum.Enum):
LINK = 'link'


@dataclass
class CompileCheckResult:

"""The result of a Compiler check.
:param result: Whether the check was successful or not
:param cached: Whether the result was retrieved from a cache, defaults to False
"""

result: bool
cached: bool = False


gnu_winlibs = ['-lkernel32', '-luser32', '-lgdi32', '-lwinspool', '-lshell32',
'-lole32', '-loleaut32', '-luuid', '-lcomdlg32', '-ladvapi32']

Expand Down Expand Up @@ -530,12 +543,12 @@ def compute_parameters_with_absolute_paths(self, parameter_list: T.List[str],
def has_members(self, typename: str, membernames: T.List[str],
prefix: str, env: 'Environment', *,
extra_args: T.Union[None, T.List[str], T.Callable[[CompileCheckMode], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
raise EnvironmentException('%s does not support has_member(s) ' % self.get_id())

def has_type(self, typename: str, prefix: str, env: 'Environment',
extra_args: T.Union[T.List[str], T.Callable[[CompileCheckMode], T.List[str]]], *,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
raise EnvironmentException('%s does not support has_type ' % self.get_id())

def symbols_have_underscore_prefix(self, env: 'Environment') -> bool:
Expand Down Expand Up @@ -604,19 +617,26 @@ def get_option_link_args(self, options: 'KeyedOptionDictType') -> T.List[str]:

def check_header(self, hname: str, prefix: str, env: 'Environment', *,
extra_args: T.Union[None, T.List[str], T.Callable[[CompileCheckMode], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
"""Check that header is usable.
Returns a two item tuple of bools. The first bool is whether the
check succeeded, the second is whether the result was cached (True)
or run fresh (False).
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
"""Check that a header is actually usable.
In contrast to :method:`has_header`, which checks that a header exists.
Tihs is useful for cases where a header contains an #error define.
:param hname: the name of the header to check for
:param prefix: A preamble to add to the top of the generated test code
:param env: The Meson Environment
:param extra_args: Any extra arguments to pass to the check, defaults to None
:param dependencies: A list of Meson Dependency objects, defaults to None
:raises EnvironmentException: If the given compiler does not implement this check
:return: a :class:`CompileCheckResult`
"""
raise EnvironmentException('Language %s does not support header checks.' % self.get_display_language())

def has_header(self, hname: str, prefix: str, env: 'Environment', *,
extra_args: T.Union[None, T.List[str], T.Callable[[CompileCheckMode], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None,
disable_cache: bool = False) -> T.Tuple[bool, bool]:
disable_cache: bool = False) -> CompileCheckResult:
"""Check that header is exists.
This check will return true if the file exists, even if it contains:
Expand All @@ -627,16 +647,14 @@ def has_header(self, hname: str, prefix: str, env: 'Environment', *,
Use check_header if your header only works in some cases.
Returns a two item tuple of bools. The first bool is whether the
check succeeded, the second is whether the result was cached (True)
or run fresh (False).
:return: a :class:`CompileCheckResult`
"""
raise EnvironmentException('Language %s does not support header checks.' % self.get_display_language())

def has_header_symbol(self, hname: str, symbol: str, prefix: str,
env: 'Environment', *,
extra_args: T.Union[None, T.List[str], T.Callable[[CompileCheckMode], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
raise EnvironmentException('Language %s does not support header symbol checks.' % self.get_display_language())

def run(self, code: 'mesonlib.FileOrString', env: 'Environment',
Expand Down Expand Up @@ -705,12 +723,10 @@ def alignment(self, typename: str, prefix: str, env: 'Environment', *,

def has_function(self, funcname: str, prefix: str, env: 'Environment', *,
extra_args: T.Optional[T.List[str]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
"""See if a function exists.
Returns a two item tuple of bools. The first bool is whether the
check succeeded, the second is whether the result was cached (True)
or run fresh (False).
:return: a :class:`CompileCheckResult`
"""
raise EnvironmentException('Language %s does not support function checks.' % self.get_display_language())

Expand Down Expand Up @@ -740,23 +756,19 @@ def get_library_naming(self, env: 'Environment', libtype: LibType,
def get_program_dirs(self, env: 'Environment') -> T.List[str]:
return []

def has_multi_arguments(self, args: T.List[str], env: 'Environment') -> T.Tuple[bool, bool]:
def has_multi_arguments(self, args: T.List[str], env: 'Environment') -> CompileCheckResult:
"""Checks if the compiler has all of the arguments.
:returns:
A tuple of (bool, bool). The first value is whether the check
succeeded, and the second is whether it was retrieved from a cache
:return: a :class:`CompileCheckResult`
"""
raise EnvironmentException(
'Language {} does not support has_multi_arguments.'.format(
self.get_display_language()))

def has_multi_link_arguments(self, args: T.List[str], env: 'Environment') -> T.Tuple[bool, bool]:
def has_multi_link_arguments(self, args: T.List[str], env: 'Environment') -> CompileCheckResult:
"""Checks if the linker has all of the arguments.
:returns:
A tuple of (bool, bool). The first value is whether the check
succeeded, and the second is whether it was retrieved from a cache
:return: a :class:`CompileCheckResult`
"""
return self.linker.has_multi_arguments(args, env)

Expand Down Expand Up @@ -948,7 +960,7 @@ def get_win_subsystem_args(self, value: str) -> T.List[str]:
# or does not target Windows
return self.linker.get_win_subsystem_args(value)

def has_func_attribute(self, name: str, env: 'Environment') -> T.Tuple[bool, bool]:
def has_func_attribute(self, name: str, env: 'Environment') -> CompileCheckResult:
raise EnvironmentException(
f'Language {self.get_display_language()} does not support function attributes.')

Expand Down Expand Up @@ -1295,21 +1307,19 @@ def compiles(self, code: 'mesonlib.FileOrString', env: 'Environment', *,
extra_args: T.Union[None, T.List[str], CompilerArgs, T.Callable[[CompileCheckMode], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None,
mode: CompileCheckMode = CompileCheckMode.COMPILE,
disable_cache: bool = False) -> T.Tuple[bool, bool]:
disable_cache: bool = False) -> CompileCheckResult:
"""Run a compilation or link test to see if code can be compiled/linked.
:returns:
A tuple of (bool, bool). The first value is whether the check
succeeded, and the second is whether it was retrieved from a cache
:return: A :class:`CompileCheckResult`
"""
with self._build_wrapper(code, env, extra_args, dependencies, mode, disable_cache=disable_cache) as p:
return p.returncode == 0, p.cached
return CompileCheckResult(p.returncode == 0, p.cached)

def links(self, code: 'mesonlib.FileOrString', env: 'Environment', *,
compiler: T.Optional['Compiler'] = None,
extra_args: T.Union[None, T.List[str], CompilerArgs, T.Callable[[CompileCheckMode], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None,
disable_cache: bool = False) -> T.Tuple[bool, bool]:
disable_cache: bool = False) -> CompileCheckResult:
if compiler:
with compiler._build_wrapper(code, env, dependencies=dependencies, want_output=True) as r:
objfile = mesonlib.File.from_absolute_file(r.output_name)
Expand Down
21 changes: 11 additions & 10 deletions mesonbuild/compilers/cpp.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
msvc_winlibs,
Compiler,
CompileCheckMode,
CompileCheckResult,
)
from .c_function_attributes import CXX_FUNC_ATTRIBUTES, C_FUNC_ATTRIBUTES
from .mixins.apple import AppleCompilerMixin
Expand Down Expand Up @@ -99,13 +100,13 @@ def get_compiler_check_args(self, mode: CompileCheckMode) -> T.List[str]:
def has_header_symbol(self, hname: str, symbol: str, prefix: str,
env: 'Environment', *,
extra_args: T.Union[None, T.List[str], T.Callable[[CompileCheckMode], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
# Check if it's a C-like symbol
found, cached = super().has_header_symbol(hname, symbol, prefix, env,
extra_args=extra_args,
dependencies=dependencies)
if found:
return True, cached
result = super().has_header_symbol(hname, symbol, prefix, env,
extra_args=extra_args,
dependencies=dependencies)
if result.result:
return result
# Check if it's a class or a template
if extra_args is None:
extra_args = []
Expand Down Expand Up @@ -186,8 +187,8 @@ class _StdCPPLibMixin(CompilerMixinBase):

def language_stdlib_provider(self, env: Environment) -> str:
# https://stackoverflow.com/a/31658120
header = 'version' if self.has_header('version', '', env)[0] else 'ciso646'
is_libcxx = self.has_header_symbol(header, '_LIBCPP_VERSION', '', env)[0]
header = 'version' if self.has_header('version', '', env).result else 'ciso646'
is_libcxx = self.has_header_symbol(header, '_LIBCPP_VERSION', '', env).result
lib = 'c++' if is_libcxx else 'stdc++'
return lib

Expand Down Expand Up @@ -609,9 +610,9 @@ def get_options(self) -> 'MutableKeyedOptionDictType':
# So we should explicitly fail at this case.
def has_function(self, funcname: str, prefix: str, env: 'Environment', *,
extra_args: T.Optional[T.List[str]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
if funcname == 'lchmod':
return False, False
return CompileCheckResult(False)
else:
return super().has_function(funcname, prefix, env,
extra_args=extra_args,
Expand Down
10 changes: 5 additions & 5 deletions mesonbuild/compilers/cuda.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
is_windows, LibType, version_compare
)
from ..options import OptionKey
from .compilers import Compiler
from .compilers import Compiler, CompileCheckResult

if T.TYPE_CHECKING:
from .compilers import CompileCheckMode
Expand Down Expand Up @@ -607,7 +607,7 @@ def sanity_check(self, work_dir: str, env: 'Environment') -> None:
def has_header_symbol(self, hname: str, symbol: str, prefix: str,
env: 'Environment', *,
extra_args: T.Union[None, T.List[str], T.Callable[[CompileCheckMode], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
if extra_args is None:
extra_args = []
fargs = {'prefix': prefix, 'header': hname, 'symbol': symbol}
Expand All @@ -621,9 +621,9 @@ def has_header_symbol(self, hname: str, symbol: str, prefix: str,
#endif
return 0;
}}'''
found, cached = self.compiles(t.format_map(fargs), env, extra_args=extra_args, dependencies=dependencies)
if found:
return True, cached
result = self.compiles(t.format_map(fargs), env, extra_args=extra_args, dependencies=dependencies)
if result.result:
return result
# Check if it's a class or a template
t = '''{prefix}
#include <{header}>
Expand Down
5 changes: 3 additions & 2 deletions mesonbuild/compilers/d.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from ..environment import Environment
from ..linkers.linkers import DynamicLinker
from ..mesonlib import MachineChoice
from .compilers import CompileCheckResult

CompilerMixinBase = Compiler
else:
Expand Down Expand Up @@ -547,7 +548,7 @@ def get_optimization_link_args(self, optimization_level: str) -> T.List[str]:
def compiler_args(self, args: T.Optional[T.Iterable[str]] = None) -> DCompilerArgs:
return DCompilerArgs(self, args)

def has_multi_arguments(self, args: T.List[str], env: 'Environment') -> T.Tuple[bool, bool]:
def has_multi_arguments(self, args: T.List[str], env: 'Environment') -> CompileCheckResult:
return self.compiles('int i;\n', env, extra_args=args)

def _get_target_arch_args(self) -> T.List[str]:
Expand Down Expand Up @@ -632,7 +633,7 @@ def alignment(self, typename: str, prefix: str, env: 'Environment', *,
def has_header(self, hname: str, prefix: str, env: 'Environment', *,
extra_args: T.Union[None, T.List[str], T.Callable[['CompileCheckMode'], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None,
disable_cache: bool = False) -> T.Tuple[bool, bool]:
disable_cache: bool = False) -> CompileCheckResult:

extra_args = self._get_compile_extra_args(extra_args)
code = f'''{prefix}
Expand Down
9 changes: 5 additions & 4 deletions mesonbuild/compilers/fortran.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from ..environment import Environment
from ..linkers.linkers import DynamicLinker
from ..mesonlib import MachineChoice
from .compilers import CompileCheckResult


class FortranCompiler(CLikeCompiler, Compiler):
Expand All @@ -47,7 +48,7 @@ def __init__(self, exelist: T.List[str], version: str, for_machine: MachineChoic

def has_function(self, funcname: str, prefix: str, env: 'Environment', *,
extra_args: T.Optional[T.List[str]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
raise MesonException('Fortran does not have "has_function" capability.\n'
'It is better to test if a Fortran capability is working like:\n\n'
"meson.get_compiler('fortran').links('block; end block; end program')\n\n"
Expand Down Expand Up @@ -105,10 +106,10 @@ def find_library(self, libname: str, env: 'Environment', extra_dirs: T.List[str]
code = 'stop; end program'
return self._find_library_impl(libname, env, extra_dirs, code, libtype, lib_prefix_warning)

def has_multi_arguments(self, args: T.List[str], env: 'Environment') -> T.Tuple[bool, bool]:
def has_multi_arguments(self, args: T.List[str], env: 'Environment') -> CompileCheckResult:
return self._has_multi_arguments(args, env, 'stop; end program')

def has_multi_link_arguments(self, args: T.List[str], env: 'Environment') -> T.Tuple[bool, bool]:
def has_multi_link_arguments(self, args: T.List[str], env: 'Environment') -> CompileCheckResult:
return self._has_multi_link_arguments(args, env, 'stop; end program')

def get_options(self) -> 'MutableKeyedOptionDictType':
Expand Down Expand Up @@ -181,7 +182,7 @@ def language_stdlib_only_link_flags(self, env: 'Environment') -> T.List[str]:
def has_header(self, hname: str, prefix: str, env: 'Environment', *,
extra_args: T.Union[None, T.List[str], T.Callable[['CompileCheckMode'], T.List[str]]] = None,
dependencies: T.Optional[T.List['Dependency']] = None,
disable_cache: bool = False) -> T.Tuple[bool, bool]:
disable_cache: bool = False) -> CompileCheckResult:
'''
Derived from mixins/clike.py:has_header, but without C-style usage of
__has_include which breaks with GCC-Fortran 10:
Expand Down
3 changes: 2 additions & 1 deletion mesonbuild/compilers/mixins/clang.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
if T.TYPE_CHECKING:
from ...environment import Environment
from ...dependencies import Dependency # noqa: F401
from ..compilers import CompileCheckResult

clang_color_args: T.Dict[str, T.List[str]] = {
'auto': ['-fdiagnostics-color=auto'],
Expand Down Expand Up @@ -112,7 +113,7 @@ def get_compiler_check_args(self, mode: CompileCheckMode) -> T.List[str]:

def has_function(self, funcname: str, prefix: str, env: 'Environment', *,
extra_args: T.Optional[T.List[str]] = None,
dependencies: T.Optional[T.List['Dependency']] = None) -> T.Tuple[bool, bool]:
dependencies: T.Optional[T.List['Dependency']] = None) -> CompileCheckResult:
if extra_args is None:
extra_args = []
# Starting with XCode 8, we need to pass this to force linker
Expand Down
Loading

0 comments on commit d31635c

Please sign in to comment.