-
Notifications
You must be signed in to change notification settings - Fork 213
Expose default values for parameters to macros? #3305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
If you can add a default value later, you can start by declaring class B extends A {
void foo([int x]);
} in phase one, then add the I'm more interested in how you give access to an existing default value to the macro, so that it can use it elsewhere, like: class C {
C([int x = _someConstant]);
} and you want to create a class: class D extends C {
C([int x = _someConstant]) : assert(x > 0), super(x);
} in another library. You can't necessarily copy expressions from one context to another, and there isn't always a way to express a constant value in another library using Dart source at all. |
This is explicitly disallowed right now (augmentations in general cannot alter default values). For new methods in an augmentation if you define them in phase one (can only do this if also declaring a type), then if you do anything to them in phase two that is an augmentation of the one defined in phase one. We could change how that works but it is much easier to think about things that way generally (each phases output is essentially an augmentation of the original library + previous phases augmentations).
Good point regarding private identifiers, this is not something we can allow. It is a general problem for overrides though. Maybe this is another reason to go with a language feature, as it does solve a general problem that isn't macro specific? |
+1 to what Lasse says. I think semantically default values are part of definition, not declaration. So maybe you should allow producing default values as part of definition phase and maybe this will simplify the resolution? This of course means that function augmentation would have to support adding arguments' default parameters; for this use case it would be fine to restrict it to only support declarations with no definition, so we are not altering default values, just delaying their generation till the definition phase where they belong. Private identifiers are painful in this respect, language feature would be nice. It doesn't help with my plans to rewrite Mockito, since I would need to grab |
I consider them more a part of the API, they are an interesting/important part of the declaration itself. It is a grey area though, I could see an argument either way. But you generally can't change the default value and consider that non-breaking, as an example.
My main concern with this is just that it isn't something we can enforce statically - so it introduces a new category of runtime error that macro authors will have to manage (if they try to add a default value but one already exists). I would much rather go with a language feature of some sort to avoid the situation entirely. |
Also from a more concrete design perspective, the definition phase currently has the property that ordering does not matter, it can be fully concurrent (at least in terms of running the macros, their results must appear in a certain order still though to get a consistent augmentation). If you can add default values, but only once, we lose that property. We also have to keep track during execution of macros whether or not a previous macro has added a default value, which adds an extra complication and cost that we don't have today. I don't think it provides enough value to be worth the complication or the runtime cost. |
Can an augmentation add a default value, if used by itself? I'd personally expect the choice of everywhere to have a default value, or at least which default value, to be a late choice, like when you choose whether the function body is |
The general principle we have been following is that augmentations cannot affect the API of functions. You should be able to use them exactly as they appear in code, and not have magic extra parameters appear (and especially not have a parameter change types or something). Whether that same logic should apply to default values is not is not very clear-cut, but the current decision is that yes it does. We do not allow adding default values at all via augmentation. Personally I think that is the correct choice, because I do think they are a part of the API contract - at least sometimes. For instance if I have this function
I think this argument is sensible, a savvy enough reader should know that there must be a default value somewhere, and a smart enough IDE plugin could even show it to you inline as if it was there. But, I don't think there is a lot of value in it. For hand written augmentations you can just as easily put it in the original function. For macros I see a lot of issues (listed in previous comments). |
Isn't the breakage orthogonal to where the default value is? void f({int? a}) {
a ??= 42;
print(a);
} |
That is a fair argument 🤷 . I am still hesitant about allowing macros to add default values though because it is difficult to enforce the "only allow it if it isn't already there" part. But we do already have some similar things, in the declarations phase. I would like to keep them at least constrained to that phase. We could also just tell macro authors to use that pattern instead of regular default values. |
cc @munificent thoughts? |
#2269 looks like it would work as the language feature if we went that route |
I do think I am pretty convinced this just needs to be a language feature. In the short term though that does mean that mockito won't be a viable target for early macro testing, unfortunately. |
I think there are two separate issues here: allowing to introspect default arguments and allowing to augment method definitions with default arguments. Please correct me if I'm wrong, but it looks like the issues are around augmentation, introspection shouldn't be a big issue, right? Also, do I understand it correctly, that problems appear only if one wants to add default values? If I try to emit a completely new method definition with some arguments having default values, that won't be a problem, even today? If these two things are true, I don't think the default value thing makes it worse for "mockito on macros". The main blocker is still getting to introspect definitions that are known to be complete (because of being in another strongly connected component) during the early stages. If we could get this + introspection of arguments' defaults, mockito can happily emit everything in the types stage. So, maybe mockito is not a viable target for macro testing, but for another reason: I can only think of one way to make it work, and this way won't exercise the phases interactions much (or at all), and you probably want that. |
Introspection is actually an issue, because of private variables. We can't give you a valid
Correct.
Unfortunately the first is not true, I don't know how we work around the private variables issue. I also don't really want to expose this API at all if long term we have a different feature in mind - although if we only exposed it experimentally that might be ok (it could be marked deprecated from day one?). But it is still some wasted effort.
For early macros it would still be valuable, it would still exercise the other parts of the system. Also knowing that something like mockito can be done in the first phase is in itself valuable. |
Right, but you should probably be able to return
Agreed. But it could probably be deprecated from day one as you say, or even not be a part of public API. My point is it's probably too early to say "#2269 will just solve this", since this new language feature was not even approved yet. Are we sure it will eventually be in the language? |
We could possibly do something like this, (probably throw if it can't be created, I wouldn't want a class like But it doesn't seem like a good long term solution so I hate to design and ship an API for a new language feature that we will have trouble removing later.
I would say today that feature is unlikely to ship unless the macro feature could make a strong argument for it existing. But, I also think that mockito is pretty unique in its requirements here. Another feature that I think we will more likely ship is |
Well it's been around for about a year, but patterns and null safety were around for about a decade, and still went ahead. ;-) Granted, it is a specialized mechanism: The need for denoting a default value generally only comes up when there is a need to do some kind of forwarding. So the question is basically whether we want support for safer/cleaner forwarding, and then how we'd do that. #2269 is one possible approach, and abstraction over actual argument lists (including the ability to pass-or-not-pass an actual argument programmatically) is another. |
I have a use case detailed in #3611 that requires the computed value of any parameter's default, in order to expose the API contract of a given function to a generated @cliCommand
void someFunction({
String value = varValue,
}) {}
const varValue = 'foo';
// -- generates: --
final someFunctionParser = ArgParser()
..addOption(
'value',
defaultsTo: 'foo',
...
); In this simplified example, the macro would create a command
I think if the macro user (function author in the above case) chooses to use a private variable as the default to a parameter, then their intent is clear and lack of introspection on that value would be okay. But for any public elements, introspecting the default value and evaluating the Code object would be expected, at least to some extent. |
What we can do depends on which requirements we need to satisfy. If we ignore that, and pretend to be generating Kernel code, not source syntax, then nothing is impossible, and we could hypothetically just say "use that constant value" without giving any way to express the value using source. That's what the compilers do today when they have forwarding factory constructors, or mixin application forwarding constructors, inherit the default value of the constructor they forward to. The compilers can and do allow "inferring" constants that cannot be expressed by user-written code in the same place. But that would give macros the ability to do something you cannot (currently) do in any other way, which would probably quickly lead to a macro just for copying default values. (I'd rather make it possible to programmatically call a function without a value, triggering the default value, without the combinatorial explosion when there are multiple optional parameters. Something like |
I hear you, that's definitely not ideal. For calling the function, I agree that conditionally passing in arguments would be the best case forward. However there's a part of the use case I'm speaking of that has nothing to do with calling the function or augmenting it - it has more to do with capturing the entire function signature for use elsewhere in the program. In the case of Similarly, I've been working on a backend code-gen feature that analyzes annotated Dart functions to generate an OpenAPI schema of the entire API, which can then be served by the annotated shelf server app. Default value computation and doc comment introspection is just as necessary there. I recognize that these are rather niche use cases, and possibly stretching the bounds of what macros are being built to solve, but they're real use cases that would otherwise require either a) continued use of |
Sorry for the very long delay. Yeah, in general, we won't be able to take an expression in one context, paste into generated code in some other context and rely on that being able to work because of privacy and name resolution. As @lrhn says, we could add some magic mechanism that macros have access to in order to enable that to work. But I'm inclined to interpret this as a signal that a language feature is worth having to "inherit" the default value from the overidden member. I'm also OK with simply saying (for now, or perhaps forever) that macros just don't have access to default value expressions and macro authors have to work around that limitation. If that means that you can't have a macro generate an override of a method that has a default value, that's OK. I suspect that most macro-generated methods won't be overrides and of the ones that are, it's reasonable to simply limit them to methods that don't have optional parameters with non-obvious default values. |
This seems like a case where it'd make sense to just adopt the long-awaited syntax like: @Macro()
void myFn({int i = 123}) {}
// generated:
void myFn2({int? i}) {
myFn(
if (i != null) i: i,
);
} The above obviously doesn't help when you have a nullable parameter with a default value, but that could probably be done with the same |
Exposing default values to macros is definitely a requirement IMO. I've already encountered cases where I have no solution to the problem I'm trying to solve because of it. A |
The macro apis today do not give access to default values for parameters (or initializer expressions for variables either).
When augmenting a function today you are not allowed to specify default values, so we probably don't need them in that case. However, when overriding a method with a new declaration in a class (so you aren't augmenting an existing method/constructor, you are specifying a new one, which is an override), you do likely need access to the default values of the original parameters.
We could likely only provide these as
Code
objects and not values (the values might be user types, have unresolved identifiers, etc). But that should be sufficient as the use case is really just plopping in the same default value.Concerns
I am concerned that in the case of unresolved identifiers (that will be produced in phase 2), we can't even always produce a valid augmentation library (at least in phase 1, although it would be challenging even in phase 2).
Consider a macro that runs in phase one, but it creates a type that extends some other type and overrides some members. Generally that would be hard to actually do since you can't introspect over members, but the macro could actually live on a member, something like this:
Possibly we could just assume that any unresolved identifier will eventually exist in the current library, and not do any prefix. But that seems sketchy.
Other options
Alternatively, could we add a way of referring to the overridden default value? This would be generally useful, and would avoid the issue entirely. Maybe something like
void foo({super.x})
orvoid foo({int x = super.default})
?The text was updated successfully, but these errors were encountered: