Skip to content

Interpreter: add a default() type for keyword arguments#11987

Open
dcbaker wants to merge 2 commits into
mesonbuild:masterfrom
dcbaker:submit/default-object
Open

Interpreter: add a default() type for keyword arguments#11987
dcbaker wants to merge 2 commits into
mesonbuild:masterfrom
dcbaker:submit/default-object

Conversation

@dcbaker

@dcbaker dcbaker commented Jul 14, 2023

Copy link
Copy Markdown
Member

This type when passed keyword argument will instruct typed_kwargs to use the default value, but in a way the user can set explicitly. typed_kwargs makes this extremely trivial to implement, but for it to be universally effective we need to have all keyword arguments be handled by typed_kwargs

@tristan957

Copy link
Copy Markdown
Member

Could you give an example where this might be useful?

Comment thread mesonbuild/interpreterbase/decorators.py
Comment thread test cases/rust/1 basic/meson.build Outdated
override_options : ['b_ndebug=false'],
),
should_fail : true,
protocol : default(),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see a unit test that verify passing default() to a new kwarg does not trigger FeatureNew/FeatureDeprecated warnings.

@xclaesse

Copy link
Copy Markdown
Member

Could you give an example where this might be useful?

One usage I would like (not sure current implementation would work) is to allow using new kwargs without triggering FeatureNew warning.

project(..., meson_version: '>=1.0.0')
val = default()
if meson.version().version_compare('>=1.2.0')
  val = 'foo'
endif
some_function(..., new_karg_from_1_2: val)

This currently would have to be done with an ugly dict:

project(..., meson_version: '>=1.0.0')
d = {}
if meson.version().version_compare('>=1.2.0')
  d += {'new_karg_from_1_2': 'foo'}
endif
some_function(..., kwargs: d)

@xclaesse

Copy link
Copy Markdown
Member

Another usage is when the default value for a kwarg is not obvious, sometimes it's platform dependent, and you optionally want to override the default. An example of such case is name_prefix: https://mesonbuild.com/Reference-manual_functions.html#build_target_name_prefix. Notice how we document totally weird empty list [] to make it default? That can be used for example as:

lib_prefix = default()
if get_option('force-lib-prefix-on-windows')
  lib_prefix = 'lib'
endif
library(..., name_prefix: lib_prefix)

@xclaesse

Copy link
Copy Markdown
Member

One usage I would like (not sure current implementation would work) is to allow using new kwargs without triggering FeatureNew warning.

Note that it's for that use-case that I had in mind to name this none() instead of default(), but whatever...

@dcbaker dcbaker force-pushed the submit/default-object branch from 75873c6 to a7ab2d8 Compare June 2, 2026 18:06
@dcbaker dcbaker marked this pull request as ready for review June 2, 2026 18:07
@dcbaker dcbaker requested a review from jpakkane as a code owner June 2, 2026 18:07
@bonzini bonzini added this to the 1.12 milestone Jun 2, 2026
@dcbaker dcbaker force-pushed the submit/default-object branch from a7ab2d8 to 5362e1d Compare June 4, 2026 19:04
@dcbaker dcbaker requested a review from mensinda as a code owner June 4, 2026 19:04
@dcbaker dcbaker force-pushed the submit/default-object branch from 5362e1d to 3b8458f Compare June 22, 2026 16:03
@bonzini

bonzini commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Looks good but I would also prefer to have a test case checking whether FeatureNew triggers if you're using default(). It's an important corner case, independent of your choice of how to implement it.

Also worth testing is:

  • the error case for required kwargs
  • how a dictionary constructor handles it, since it reuses ArgumentsNode and that has surprising effects with kwargs. Including checking {'kwargs': default()}.
  • what is the effect of passing default() as a positional argument, both as a required one, as the second argument to set_variable(), or as part of a flattened one.
  • what is the effect of passing kwargs: default().

Overall it's probably better to have a dedicated entry in test cases/common/.

@dcbaker dcbaker force-pushed the submit/default-object branch from 3b8458f to c30e8a6 Compare June 26, 2026 05:38
@dnicolodi

dnicolodi commented Jun 26, 2026

Copy link
Copy Markdown
Member

Why is a default() function better than a nil (or whatever we want to call it) singleton? I see the probability of clashing with existing names in user meson.build very similar, if not higher for default.

Comment thread test cases/common/1 trivial/meson.build Outdated
dependencies: '')
assert(exe.name() == 'trivialprog')
test('runtest', exe) # This is a comment
test('runtest', exe)) # This is a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something went wrong here.

@dcbaker

dcbaker commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

I don't like nil because now we're introducing the optional union type, like python's T | None, which is just a pain in the butt to make really correct. The value of default() is that it's a clear solution to the problem of "I want to override this option in some cases", like the case of library(prefix : host_machine.system() == 'windows' ? '' : default(). And unlike a Null type it's self documenting that it sets a value to default. It also mirrors the disabler() pattern.

If we're worried about name collisions, maybe default_value() would be better?

dcbaker added 2 commits June 26, 2026 15:35
This gives a way to conditionally override a keyword argument, such as
when setting a value specifically for one platform. For example,
```meson
library(
  ...,
  name_prefix : host_machine.system() == 'windows' ? '' : default(),
)
```
We have the `default()` function now, so we don't need this.
@dcbaker dcbaker force-pushed the submit/default-object branch from c30e8a6 to 9c520a9 Compare June 26, 2026 22:36
@dnicolodi

Copy link
Copy Markdown
Member

I don't like nil because now we're introducing the optional union type, like python's T | None, which is just a pain in the butt to make really correct.

I don't understand this. Don't you need the same union type for the DefaultObject object added here?

The value of default() is that it's a clear solution to the problem of "I want to override this option in some cases", like the case of library(prefix : host_machine.system() == 'windows' ? '' : default().

library(prefix : host_machine.system() == 'windows' ? '' : nil looks the same to me.

Going back to the previous point, library() would need to be updated to accept str | DefaultObject for the prefix argument, which would be str | nil if we go for a nil object instead. I must be missing something.

The only reason for bringing up nil as an alternative spelling for default() is that the concept of nil / None / NULL may be familiar to most. Semantically speaking nil is the empty list in lisp, and it maps nicely to the fact that now [] is used in some places to indicate the absence of an argument 😄


name_prefix:
type: str | array[void]
type: str

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be str | default strictly speaking? This is what I meant with the other comment. I don't see much of a difference in typing annotations between introducing default() and some form of nil / null / none.

@dcbaker

dcbaker commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

The only reason for bringing up nil as an alternative spelling for default() is that the concept of nil / None / NULL may be familiar to most. Semantically speaking nil is the empty list in lisp, and it maps nicely to the fact that now [] is used in some places to indicate the absence of an argument 😄

My concern is that in Python especially None doesn't have a set semantic meaning. It gets used to mean all kinds of things. I could mean "no argument", "use a default", "I'm avoiding eager binding issues", "I have a tri-state but don't want to use an enum", "I'm returning an error", "I couldn't find a value in a mapping", "this doesn't have to have a value.", "this value is uninitialized", etc.

I like default() because it's clear this is only for getting a default argument. It does one thing.

Shouldn't this be str | default strictly speaking?

We don't document that every argument can be T | disabler(), so there's some prior art for not documenting a special type in every function call.

@dnicolodi

Copy link
Copy Markdown
Member

It is not required for the nil singleton to implement the same semantics as Python's None. I used nil and not none as an example with the precise intent to indicate that it is something different.

I like default() because it's clear this is only for getting a default argument. It does one thing.

Unless comparing a value to default() is not allowed (I haven't checked whether this patch adds or not this capability), I am sure users will abuse default() for most of the same nefarious things.

@dcbaker

dcbaker commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

Unless comparing a value to default() is not allowed (I haven't checked whether this patch adds or not this capability), I am sure users will abuse default() for most of the same nefarious things.

comparison of unlike types became a hard error in 0.60, so that's not allowed :). That's why we have the is_disabler() function.

@dnicolodi

Copy link
Copy Markdown
Member

comparison of unlike types became a hard error in 0.60, so that's not allowed :). That's why we have the is_disabler() function.

Then we can have a nil object and no one can use it to do any of the nefarious things that you mentioned 😄 (at this point I'm just kidding).

@dcbaker

dcbaker commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

I'm willing to be flexible on the name, but my experience of having a None (and NULL, nullptr, etc) types is that unless you're going to go having a sum/adt type like rust's Option or C++'s std::optional then it just makes things much more difficult to use correctly, and Meson's design is pretty much the opposite of flexible sum types.

@bonzini

bonzini commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

I agree that it's better to make this a special thing for argument passing (which is by far the most magic part of meson between flattening, disablers and kwargs) that to try to generalize it.

@dnicolodi

Copy link
Copy Markdown
Member

My questions do not concern the semantics, just the naming of what is being introduced. I am not sure whether a default() function is easier or harder to understand at first sight that a nil value.

It may be just me, but seeing a default() function for the first time, I would expect it to return an object with a more complex behavior than just resolving to the default argument for a parameter to another function. On the other hand, seeing a variable initialized to nil makes it clear that it is a placeholder value I should not bother investigating much.

If there is consensus that default() is self explanatory, I will consider this debated more than enough 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants