Skip to content

Commit

Permalink
Remove option type from OptionKey and get it from OptionStore instead.
Browse files Browse the repository at this point in the history
  • Loading branch information
jpakkane committed Jul 17, 2024
1 parent 978a58e commit de8e3d6
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 60 deletions.
22 changes: 15 additions & 7 deletions mesonbuild/coredata.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
pickle_load
)

from .options import OptionKey, OptionType
from .options import OptionKey

from .machinefile import CmdLineFileParser

Expand Down Expand Up @@ -586,8 +586,6 @@ def get_external_link_args(self, for_machine: MachineChoice, lang: str) -> T.Lis

def update_project_options(self, project_options: 'MutableKeyedOptionDictType', subproject: SubProject) -> None:
for key, value in project_options.items():
if not self.optstore.is_project_option(key):
continue
if key not in self.optstore:
self.optstore.add_project_option(key, value)
continue
Expand Down Expand Up @@ -654,7 +652,7 @@ def set_options(self, opts_to_set: T.Dict[OptionKey, T.Any], subproject: str = '
continue
elif k in self.optstore:
dirty |= self.set_option(k, v, first_invocation)
elif k.machine != MachineChoice.BUILD and k.type != OptionType.COMPILER:
elif k.machine != MachineChoice.BUILD and not self.optstore.is_compiler_option(k):
unknown_options.append(k)
if unknown_options:
unknown_options_str = ', '.join(sorted(str(s) for s in unknown_options))
Expand Down Expand Up @@ -700,9 +698,9 @@ def set_default_options(self, default_options: T.MutableMapping[OptionKey, str],
continue
# Skip base, compiler, and backend options, they are handled when
# adding languages and setting backend.
if k.type in {OptionType.COMPILER, OptionType.BACKEND}:
if self.optstore.is_compiler_option(k) or self.optstore.is_backend_option(k):
continue
if k.type == OptionType.BASE and k.as_root() in base_options:
if self.optstore.is_base_option(k) and k.as_root() in base_options:
# set_options will report unknown base options
continue
options[k] = v
Expand Down Expand Up @@ -908,7 +906,17 @@ def __getitem__(self, key: OptionKey) -> options.UserOption:
# FIXME: This is fundamentally the same algorithm than interpreter.get_option_internal().
# We should try to share the code somehow.
key = key.evolve(subproject=self.subproject)
if not key.is_project_hack_for_optionsview():
if not isinstance(self.original_options, options.OptionStore):
# This is only used by CUDA currently.
# This entire class gets removed when option refactor
# is finished.
if '_' in key.name or key.lang is not None:
is_project_option = False
else:
sys.exit(f'FAIL {key}.')
else:
is_project_option = self.original_options.is_project_option(key)
if not is_project_option:
opt = self.original_options.get(key)
if opt is None or opt.yielding:
key2 = key.as_root()
Expand Down
76 changes: 31 additions & 45 deletions mesonbuild/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from itertools import chain
from functools import total_ordering
import argparse
import enum

from .mesonlib import (
HoldableObject,
Expand Down Expand Up @@ -39,32 +38,6 @@
buildtypelist = ['plain', 'debug', 'debugoptimized', 'release', 'minsize', 'custom']


class OptionType(enum.IntEnum):

"""Enum used to specify what kind of argument a thing is."""

BUILTIN = 0
BACKEND = 1
BASE = 2
COMPILER = 3
PROJECT = 4

def _classify_argument(key: 'OptionKey') -> OptionType:
"""Classify arguments into groups so we know which dict to assign them to."""

if key.name.startswith('b_'):
return OptionType.BASE
elif key.lang is not None:
return OptionType.COMPILER
elif key.name in _BUILTIN_NAMES or key.module:
return OptionType.BUILTIN
elif key.name.startswith('backend_'):
assert key.machine is MachineChoice.HOST, str(key)
return OptionType.BACKEND
else:
assert key.machine is MachineChoice.HOST, str(key)
return OptionType.PROJECT

# This is copied from coredata. There is no way to share this, because this
# is used in the OptionKey constructor, and the coredata lists are
# OptionKeys...
Expand Down Expand Up @@ -117,21 +90,19 @@ class OptionKey:
internally easier to reason about and produce.
"""

__slots__ = ['name', 'subproject', 'machine', 'lang', '_hash', 'type', 'module']
__slots__ = ['name', 'subproject', 'machine', 'lang', '_hash', 'module']

name: str
subproject: str
machine: MachineChoice
lang: T.Optional[str]
_hash: int
type: OptionType
module: T.Optional[str]

def __init__(self, name: str, subproject: str = '',
machine: MachineChoice = MachineChoice.HOST,
lang: T.Optional[str] = None,
module: T.Optional[str] = None,
_type: T.Optional[OptionType] = None):
module: T.Optional[str] = None):
# the _type option to the constructor is kinda private. We want to be
# able tos ave the state and avoid the lookup function when
# pickling/unpickling, but we need to be able to calculate it when
Expand All @@ -142,9 +113,6 @@ def __init__(self, name: str, subproject: str = '',
object.__setattr__(self, 'lang', lang)
object.__setattr__(self, 'module', module)
object.__setattr__(self, '_hash', hash((name, subproject, machine, lang, module)))
if _type is None:
_type = _classify_argument(self)
object.__setattr__(self, 'type', _type)

def __setattr__(self, key: str, value: T.Any) -> None:
raise AttributeError('OptionKey instances do not support mutation.')
Expand All @@ -155,7 +123,6 @@ def __getstate__(self) -> T.Dict[str, T.Any]:
'subproject': self.subproject,
'machine': self.machine,
'lang': self.lang,
'_type': self.type,
'module': self.module,
}

Expand All @@ -173,8 +140,8 @@ def __setstate__(self, state: T.Dict[str, T.Any]) -> None:
def __hash__(self) -> int:
return self._hash

def _to_tuple(self) -> T.Tuple[str, OptionType, str, str, MachineChoice, str]:
return (self.subproject, self.type, self.lang or '', self.module or '', self.machine, self.name)
def _to_tuple(self) -> T.Tuple[str, str, str, MachineChoice, str]:
return (self.subproject, self.lang or '', self.module or '', self.machine, self.name)

def __eq__(self, other: object) -> bool:
if isinstance(other, OptionKey):
Expand All @@ -199,7 +166,7 @@ def __str__(self) -> str:
return out

def __repr__(self) -> str:
return f'OptionKey({self.name!r}, {self.subproject!r}, {self.machine!r}, {self.lang!r}, {self.module!r}, {self.type!r})'
return f'OptionKey({self.name!r}, {self.subproject!r}, {self.machine!r}, {self.lang!r}, {self.module!r})'

@classmethod
def from_string(cls, raw: str) -> 'OptionKey':
Expand Down Expand Up @@ -269,7 +236,8 @@ def as_host(self) -> 'OptionKey':

def is_project_hack_for_optionsview(self) -> bool:
"""This method will be removed once we can delete OptionsView."""
return self.type is OptionType.PROJECT
import sys
sys.exit('FATAL internal error. This should not make it into an actual release. File a bug.')


class UserOption(T.Generic[_T], HoldableObject):
Expand Down Expand Up @@ -712,6 +680,7 @@ def add_to_argparse(self, name: str, parser: argparse.ArgumentParser, help_suffi
class OptionStore:
def __init__(self):
self.d: T.Dict['OptionKey', 'UserOption[T.Any]'] = {}
self.project_options = set()

def __len__(self):
return len(self.d)
Expand All @@ -734,6 +703,7 @@ def add_system_option(self, key: T.Union[OptionKey, str], valobj: 'UserOption[T.
def add_project_option(self, key: T.Union[OptionKey, str], valobj: 'UserOption[T.Any]'):
key = self.ensure_key(key)
self.d[key] = valobj
self.project_options.add(key)

def set_value(self, key: T.Union[OptionKey, str], new_value: 'T.Any') -> bool:
key = self.ensure_key(key)
Expand Down Expand Up @@ -774,23 +744,39 @@ def get(self, *args, **kwargs) -> UserOption:

def is_project_option(self, key: OptionKey) -> bool:
"""Convenience method to check if this is a project option."""
return key.type is OptionType.PROJECT
return key in self.project_options

def is_reserved_name(self, key: OptionKey) -> bool:
return not self.is_project_option(key)
if key.name in _BUILTIN_NAMES:
return True
# FIXME, this hack is needed until the lang field is removed from OptionKey.
if key.lang is not None:
return True
if '_' not in key.name:
return False
prefix = key.name.split('_')[0]
# Pylint seems to think that it is faster to build a set object
# and all related work just to test whether a string has one of two
# values. It is not, thank you very much.
if prefix in ('b', 'backend'): # pylint: disable=R6201
return True
from .compilers import all_languages
if prefix in all_languages:
return True
return False

def is_builtin_option(self, key: OptionKey) -> bool:
"""Convenience method to check if this is a builtin option."""
return key.type is OptionType.BUILTIN
return key.name in _BUILTIN_NAMES or key.module

def is_base_option(self, key: OptionKey) -> bool:
"""Convenience method to check if this is a base option."""
return key.type is OptionType.BASE
return key.name.startswith('b_')

def is_backend_option(self, key: OptionKey) -> bool:
"""Convenience method to check if this is a backend option."""
return key.type is OptionType.BACKEND
return key.name.startswith('backend_')

def is_compiler_option(self, key: OptionKey) -> bool:
"""Convenience method to check if this is a compiler option."""
return key.type is OptionType.COMPILER
return key.lang is not None
2 changes: 1 addition & 1 deletion unittests/allplatformstests.py
Original file line number Diff line number Diff line change
Expand Up @@ -3758,9 +3758,9 @@ def test_summary(self):
User defined options
backend : ''' + self.backend_name + '''
enabled_opt : enabled
libdir : lib
prefix : /usr
enabled_opt : enabled
python : ''' + sys.executable + '''
''')
expected_lines = expected.split('\n')[1:]
Expand Down
14 changes: 7 additions & 7 deletions unittests/internaltests.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
LibType, MachineChoice, PerMachine, Version, is_windows, is_osx,
is_cygwin, is_openbsd, search_version, MesonException,
)
from mesonbuild.options import OptionKey, OptionType
from mesonbuild.options import OptionKey
from mesonbuild.interpreter.type_checking import in_set_validator, NoneType
from mesonbuild.dependencies.pkgconfig import PkgConfigDependency, PkgConfigInterface, PkgConfigCLI
from mesonbuild.programs import ExternalProgram
Expand Down Expand Up @@ -1704,16 +1704,16 @@ def test_major_versions_differ(self) -> None:

def test_option_key_from_string(self) -> None:
cases = [
('c_args', OptionKey('args', lang='c', _type=OptionType.COMPILER)),
('build.cpp_args', OptionKey('args', machine=MachineChoice.BUILD, lang='cpp', _type=OptionType.COMPILER)),
('prefix', OptionKey('prefix', _type=OptionType.BUILTIN)),
('made_up', OptionKey('made_up', _type=OptionType.PROJECT)),
('c_args', OptionKey('args', lang='c')),
('build.cpp_args', OptionKey('args', machine=MachineChoice.BUILD, lang='cpp')),
('prefix', OptionKey('prefix')),
('made_up', OptionKey('made_up')),

# TODO: the from_String method should be splitting the prefix off of
# these, as we have the type already, but it doesn't. For now have a
# test so that we don't change the behavior un-intentionally
('b_lto', OptionKey('b_lto', _type=OptionType.BASE)),
('backend_startup_project', OptionKey('backend_startup_project', _type=OptionType.BACKEND)),
('b_lto', OptionKey('b_lto')),
('backend_startup_project', OptionKey('backend_startup_project')),
]

for raw, expected in cases:
Expand Down

0 comments on commit de8e3d6

Please sign in to comment.