Dictionary expansion for keyword arguments

[Akaricchi]

This is a basic draft for an implementation of #3819. It should be considered an RFC more than anything.

A python-like syntax is chosen in this implementation:

project('kwargs', 'c')

opts = { 'install' : true }
exe = executable('foobar', 'foobar.c', *opts)

The above is equivalent to:

project('kwargs', 'c')

exe = executable('foobar', 'foobar.c', install : true)

Unlike Python, I went for a single asterisk. This is because Python has both positional and keyword arguments expansion (* and ** respectively). The former does not make much sense for meson, and the single asterisk syntax is otherwise unambiguous.

As in Python, the expansion must be the last element in the argument list:

project('kwargs', 'c')

opts = { 'install' : true }
exe = executable('foobar', 'foobar.c', *opts, 'some_option' : 'some_value')
# Parse error

Keywords specified in the dictionary override those written directly in the call:

project('kwargs', 'c')

opts = { 'install' : true }
exe = executable('foobar', 'foobar.c', install : false, *opts)
# install is true

The expansion expression can be arbitrarily complex (at least, as far as meson's parser allows):

project('kwargs', 'c')

opts = { 'install' : true }
new_opts = { 'install' : false, 'super_install' : true }

exe = executable('foobar', 'foobar.c', 
    install : false,
    *(meson.version().version_compare('>0.99.0') ? new_opts : opts)
)

An implementation detail allows for a certain peculiarity with this syntax: the asterisk "operator" has the lowest precedence, so the parentheses are actually not necessary in the above example, which may not be obvious:

project('kwargs', 'c')

opts = { 'install' : true }
new_opts = { 'install' : false, 'super_install' : true }

exe = executable('foobar', 'foobar.c', 
    install : false,
    *meson.version().version_compare('>0.99.0') ? new_opts : opts
)

EDIT: Just checked, turns out Python handles it the same way.

As I mentioned on IRC earlier, this feature would be a lot more useful if we had some way of either modifying dictionaries, or creating a new one by merging two others. The later approach is consistent with the way arrays work.

[MathieuDuponchelle]

This is because Python has both positional and keyword arguments expansion (* and ** respectively). The former does not make much sense for meson

That's arguable, and we should IMHO be a hundred percent sure about that before picking a syntax that close yet different to python's.

Consider the following use case:

conf = configuration_data()
things_to_check = [['conf_name', ['foo', 'bar'], {'baz': 'foobar'}]]

foreach thing: things_to_check
  conf.set10(thing[0], check(*thing[1], **thing[2]))
endforeach

I think this could prove useful, though we don't necessary want to support that for now, I would err towards the side of caution and use ** as the dict expansion keyword in case we indeed decide we want * to expand arrays in the future :)

Apart from that, I would indeed want the feature, +1 for me :D

[Akaricchi]

As far as I know, all meson functions that take positional arguments also accept lists for those and flatten them automatically. Is there a real counterexample to that, or a potential non-abstract use case that couldn't be handled the same way?

[MathieuDuponchelle]

As far as I know, all meson functions that take positional arguments also accept lists for those and flatten them automatically

That's not always the case, see all functions with the noArgsFlattening decorator :)

[Akaricchi]

Okay, I just took a look at them. All of them fall into one of these groups:

1. Functions that take a small, fixed amount of parameters (1-2).
2. "get"-style functions with an optional "default value" parameter.

Every use case for list expansions that I can think of is not applicable to either of those (in the sense that it doesn't achieve anything you can't easily do already). In Python it's mostly used to deal with variadic functions, which all flatten their arguments in Meson.

Don't get me wrong though, I'm not religiously opposed to changing the syntax. But I'm a bit reluctant to add that extra asterisk without a solid reason to.

[MathieuDuponchelle]

Don't get me wrong though, I'm not religiously opposed to changing the syntax. But I'm a bit reluctant to add that extra asterisk without a solid reason to.

I have two reasons for this:

• better safe than sorry
• if we are to copy python's syntax, I'd rather copy it strictly

On the other hand, afaiu the reason why python has both * and ** is to be able to differentiate args from kwargs in function prototypes. As meson doesn't allow defining functions, that differentiation is not needed, I think the same operator could be used for expanding arrays and dicts, as meson will be able to expand things appropriately.

IMHO, it would probably be a good idea not to use * at all, but another operator, to avoid any confusion, what that operator should be I'm not sure :)

@jpakkane , care to chime in?

[jpakkane]

For dictionary updating we could probably do the same as for arrays. So you could do an union like this:

joined = dict1 + dict2

Or you could do this:

dict_thing += {'key': 'value'}

Which would create a new dict in the same way as += works for arrays.

For passing the keyword dict I really don't like * or **. They make sense for Python, which is a full fledged programming language but not really for Meson. Ideally we should have a mechanism that is self-explanatory because by default it is not clear what should happen if a key is both in the argument list and the kwarg dict. However if you do this:

executable('foo', 'foo.c', some_option: 'foo', default_kwargs: some_dict)

then it is clear that arguments override the values in the dictionary.

On the other hand if we consider that this will most probably be used to specify different "target classes" we could do this:

executable('foo', 'foo.c', some_option: 'foo', template: some_dict)

where template could also be called default_template or some other term. Again it would be clearer at a glance what is happening than with just *kwargs.

[ePirat]

This looks interesting, I do something similar (manually) for VLC modules but the other way around:

https://code.videolan.org/GSoC2018/ePirat/vlc/blob/meson/modules/meson.build#L36

[xclaesse]

I implemented dict + and += in #3905. Note that python itself doesn't support that, but I don't think we have to follow python here.

[jpakkane]

There is a big question here on how we do overriding. As an example you might want to use this to have a include_directory in your dict and then use that in a target. Semantically what you probably want is to add the user specified include_directories kwargs to the ones in the dict. But there are other cases where you want to override the ones in the dict.

[Akaricchi]

I think overriding should be the default behavior. It is the most consistent and unambiguous. Just think about the can of worms you open when you consider that lists and dictionaries can be nested arbitrarily deeply. You can still satisfy the include_directories use-case with these semantics, albeit more verbosely:

dict1 = { 'include_directories' : include_directories('foo'), 'some_list' : ['eggs'] }
dict2 = { 'include_directories' : include_directories('bar'), 'some_list': ['spam'] }
merged_dict = dict1 + dict2 + { 'include_directories' : dict1['include_directories'] + dict2['include_directories'] }

[Akaricchi]

@jpakkane @nirbheek so what do I do about this?

[jpakkane]

Sorry for the delay, I've been busy with CppCon. The basic idea of this is sound and we should merge something like this definitely. I'll try to get a full review done within a day or two.

[jpakkane]

Sorry for the delay, but let's see if we can come to an agreement on how this should behave.

Suppose you have a dict with some default values you'd like to use. Call it def. Then you'd use it something like this:

executable('foo', 'foo.c',
  c_args : '-DSOMETHING',
  default_kwargs : def)

Here def would be used for all kwargs except c_args (if it had it). Then if you wanted to append c_args you'd do something like this:

current_def = def + {'c_args': def['c_args'] + ['-DSOMETHING']}
executable('foo', 'foo.c',
  default_kwargs : current_def)
)

Would this be a reasonable UI? Would you need some other kind of helper functionality? The additive dict model seems (to me at least) to work fairly well for this usage.

[xclaesse]

This would be implemented for all function/methods calls in a generic way I guess? That shouldn't be too hard, basically:

for k, v in kwargs.get('default_kwargs', {})
  if k not in kwargs:
    kwargs[k] = v

Looks good to me.

One possible caveat is it could get a bit confusing with #4159, where we have kwargs that take similar dict like {'static_library' : ['-DSOMETHING']}. But I don't think it's too bad.

[ePirat]

Together with #4159 I think this will be very confusing. But to be honest, I am not a big fan of #4159 as it alone already makes things quite confusing.

[dcbaker]

this feels like it has a lot of overlap with declare_dependency, though they are subtly different (one is extends and one replaces).

[xclaesse]

@ePirat it fix real and important use-cases. If you have a better idea to fix them please comment on that PR. Otherwise I think it's ready to be merged.

[ePirat]

But declare_dependency does not work for cases in which default_kwargs will work, additionally as the name says, declare_dependency is used to declare a dependency, which is not always what you want to do but still want to provide defaults for arguments.

[dcbaker]

I'm just concerned that this is going to create a case of two ways to do 90% of the same thing, but with subtly different behavior that will work in some cases by not others,

take @jpakkane's second example:

current_def = def + {'c_args': def['c_args'] + ['-DSOMETHING']}
executable('foo', 'foo.c',
  default_kwargs : current_def)
)

we could rewrite this as:

defaults = declare_dependency(arguments : ['-DDEFAULT'])
executable('foo', 'foo.c',
  c_args : ['-DSOMETHING'],
  dependencies : defaults,
)

but it's easy to come up with cases where they wouldn't be equivalent. I'm concerned this is going to create confusion and lots of "works for me" kind of bugs. At the very least we need to have some documentation clearly explaining the differences (because they are subtle in some cases) and guidance on when to use one vs the other.

[xclaesse]

Thinking a bit more about this, I think we should raise exception in the case a kwarg is set and is also in default_kwargs, otherwise args can too easily be silently ignored, leading to hard to debug issues. That's also the behaviour of python d={'arg' : 1} foo(arg=2, **d) raise TypeError: foo() got multiple values for keyword argument 'arg'.

If we do that, it wouldn't be "default" values anymore, so I would rename default_kwargs to just kwargs.

[jpakkane]

Seems reasonable. Being more strict is nice, because we can loosen it later if it is deemed useful.

[jpakkane]

This is being done in #4578 so closing.