Skip to content
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

Allow collection if in argument lists #2316

Open
gaaclarke opened this issue Jun 27, 2022 · 15 comments
Open

Allow collection if in argument lists #2316

gaaclarke opened this issue Jun 27, 2022 · 15 comments

Comments

@gaaclarke
Copy link

gaaclarke commented Jun 27, 2022

It's inconsistent that Dart allows conditional expressions in collection literals when building Lists, but not in parameter lists. Parameters are just collections that are used to invoke a procedure.

I propose we fill out collection if's implementation to support parameter lists.

example:

Widget makeWidget(bool isFoo) =>
  Bar(
    a: 1,
    b: 2,
    if (isFoo) {
      c: 3
    },
  );

I realize there are existing ways to express this, my proposal is solely based on making the language consistent. I've written constructs like above only to remind myself that collection if only works in certain contexts. It is more glaring in Flutter applications where the tree of your application is represented in a large statement composed of constructors and collections and collection if only works sometimes.

@ghost
Copy link

ghost commented Jun 27, 2022

I believe this is asking for something similar to the second example in dart-lang/sdk#57203.

@gaaclarke
Copy link
Author

I believe this is asking for something similar to the second example in dart-lang/sdk#57203.

Yep, if you solved that you would solve what I'm asking for in the example code. Technically this is asking for something broader.

That other issue makes a good point that while we have existing syntax for representing the above example code, it requires duplicating or naming the default value which isn't ideal.

Widget makeWidget(bool isFoo) =>
  Bar(
    a: 1,
    b: 2,
    c: isFoo ? 3 : defaultValueDuplicated,
  );

I was thinking of keyword parameters when I wrote this but it's general enough to cover collection if for positional arguments as well. That seems doable if you say that one conditional expression that always evaluates to a value can replace one positional argument.

Widget makeWidget(bool isFoo) =>
  Bar(
    if (isFoo) {
      1
    }, // <-- Compilation error for signature mismatch in some cases.
    2,
    3,
  );

Widget makeWidget(bool isFoo) =>
  Bar(
    if (isFoo) {
      1
    } else {
      2
    }, // <-- This is fine.
    2,
    3,
  );

@eernstg
Copy link
Member

eernstg commented Jun 27, 2022

With #2269, we could do this:

Widget makeWidget(bool isFoo) =>
  Bar(
    a: 1,
    b: 2,
    c: isFoo ? 3 : default,
  );

default is the actual syntax that denotes the default value of the named parameter c of the Bar constructor. It is possible to denote the default value of any parameter that has a default value, in any function declaration, but the plain keyword default (where we don't specify the function or the parameter) infers the function and the parameter from the location where it occurs. As long as we just want to make an actual argument work exactly like not passing the parameter at all, a plain default will do.

This is more powerful than changing the language such that null works exactly like not passing the argument, e.g., forwarding can tailor the placement of parameters in the parameter list:

void foo([int x = 15, int y = 231]) {...}

// Forward to `foo`, but swap the argument order.
void oof([int y = foo.default[1], int x = foo.default[0]]) => foo(x, y);

void main() {
  oof(null, null); // Compile-time error: Nullable types do not occur anywhere.
  oof(default, -15); // Allows us to "omit" the first optional positional parameter.
}

Note that we don't introduce any nullable parameter types at all, and hence we preserve the static type checking that prevents nulls at call sites. We are relying on default values that are (1) different from null, and (2) preserved faithfully in the forwarding call, without any duplication of code.

@Cat-sushi
Copy link

I prefer dart-lang/linter#2232

@eernstg
Copy link
Member

eernstg commented Jun 28, 2022

@Cat-sushi, do you have an idea how you could then maintain null safety for optional parameters? That is, how would you help developers avoiding to pass null by accident, because they didn't remember that the actual argument is a nullable expression, not because they wanted to "omit that argument" by passing null?

@lrhn
Copy link
Member

lrhn commented Jun 28, 2022

The advantage of the default approach (or dart-lang/linter#2232) is that you always have the parameter, you just sometimes don't have a value.
That avoids tricky bits like:

foo(
  if (cond) x: 3,
  y: expr,
  if (!cond) x: "value",
)

Here you seem to have the same named argument twice. Because cond and !cond are (presumably) mutually exclusive, you'll only ever end up with one argument. Until cond changes during evaluation of expr, and you do have two.
Type inference should consider both?

So, you must only ever have one argument for each parameter, even a conditional one. The x: cond ? 3 : "value" or x: cond ? 3 : default syntaxes makes it clear that this is definitely the x argument, it's only the values which is conditional.

For positional arguments, we definitely do not want to move positions of arguments if an earlier one is missing.
That means a false if (cond) value as a positional argument will omit sending a value for that position, but still keep the position of later positional arguments.

My proposal for something like this syntax would be to allow if for values only:

foo(
  1,
  if (cond) 42,
  3,
  x: if (cond) "bop"
);

Here the values of the second positional and the x-named arguments are possibly missing.
That is only allowed if those parameters are optional. Otherwise it's a compile-time error.
If the condition is false, the arguments are omitted. If true, their values are passed.
You can also use an else, but then it's no different from using ?/: (although, see dart-lang/sdk#58255).

This allows one new thing: Omitting a non-trailing positional argument.
Probably not a big issue, but some functions might need to be updated to say "don't do that", or be slightly more defensively written.

Grammar:

argument ::= (identifier `:`)? argumentExpression
argumentExpression ::= 
      expression
    | `if` `(` expression `)` argumentExpression (`else` argumentExpression)?

@gaaclarke
Copy link
Author

Here you seem to have the same named argument twice. Because cond and !cond are (presumably) mutually exclusive, you'll only ever end up with one argument. Until cond changes during evaluation of expr, and you do have two.
Type inference should consider both?

You could disallow optional arguments from showing up in more than one argument's expression.

foo(
  if (cond) x: 3,
  y: expr,
  if (!cond) x: "value",  // <-- Error "x:" is assigned in 2 separate expressions.
)
foo(
  if (cond) 
    x: 3
  else
    x: "value,  // <-- Good.
  y: expr,
)

My proposal for something like this syntax would be to allow if for values only:

That sounds reasonable. My only criticism is that it isn't as close to the collection if syntax as this proposal is. That was the main problem I was trying to address, 2 different formal systems for 2 different contexts all in one giant UI declaration for technically the same thing (ie a parameter list is a collection literal).

@eernstg
Copy link
Member

eernstg commented Jun 29, 2022

Just to compare, assuming dart-lang/sdk#58240. Here is a tricky example from this comment:

foo(
  1,
  if (cond) 42,
  3,
  x: if (cond) "bop"
);

This would allow us to omit the second optional positional argument even though we're passing the third one (presumably by passing the default value or null), and it makes passing the named parameter x conditional (so we're assuming that we can have both positional and named optionals ;-). With default we would do that as follows:

foo(
  1,
  cond ? 42 : default,
  3,
  x: cond ? "bop" : default,
);

This approach may be slightly more verbose than approaches where we actually introduce a mechanism that will omit the parameter under certain circumstances, but I think it does have a simpler semantics, and hence it may be more readable.

And this one from here:

foo(
  if (cond) x: 3,
  y: expr,
  if (!cond) x: "value",  // <-- Error "x:" is assigned in 2 separate expressions.
)

// OR

foo(
  if (cond) 
    x: 3
  else
    x: "value",  // <-- Good.
  y: expr,
)

This would use cond to determine which value to pass to the named parameter x. I'd do that as follows:

foo(
  x: cond ? 3 : "value",
  y: expr,
)

@lrhn
Copy link
Member

lrhn commented Jun 29, 2022

My issue with default is that it looks like an expression, but can't actually be used everywhere an expression can. You can't do foo(x: default + 1). (Or can you? Dum-dum-DUM!)

The reason you can omit an entire entry in a map literal is that the map's type does not depend on the presence or absence of keys. Argument lists, and records, do.
I'd never allow a record expression like:

var r = (1, 2, x: 4, if (cond) y: 5);

because the type of r depends on the value of cond. That's dependent typing.

Argument lists are not records, they either have a "context type" (the function's parameter list) or it's a dynamic call anyway. In the former case, we know the parameters that are not getting an argument.
Still, I would like to enforce that the shape of an argument list is known at compile-time. It allows AoT compilers to target a specific signature which is known at compile-time, and not an exponential set of possible signatures, in case there are multiple conditional arguments.

@eernstg
Copy link
Member

eernstg commented Jun 29, 2022

My issue with default is that it looks like an expression, but can't actually be used everywhere an expression can

I don't see a problem in that: x looks like an expression, but it can't be used everywhere where an expression can be used, super.foo() has a similar property, and so on.

foo(x: default + 1) means foo(x: foo[#x].default + 1) if foo denotes a function declaration that takes a named parameter with the name x. The function and the parameter name are taken by default from the location where default occurs, but you can always choose to write it in full.

There's no limitation on the placement of an expression of the form default, f.default, C.m.default, f[k].default, and so on, they are simply a different syntax that denotes a constant expression.

I'd never allow a record expression like: var r = (1, 2, x: 4, if (cond) y: 5);
because the type of r depends on the value of cond. That's dependent typing.

This is exactly the reason why I think the approach where "passing or not passing an argument" is using default is simpler to understand. It simply doesn't omit the parameter at all, it just provides the default value such that it works just like omitting the parameter (which is true when that default value is non-null and when it is null).

So that would be an argument in favor of using default, because that approach will never omit a parameter and hence you will never have to worry about dependent types in that sense.

@mmcdon20
Copy link

IMO, collection if would make more sense in parameter lists as part of varargs (if it is ever implemented). With varargs you could potentially support all of the collection operations, including collection for and spreads ... not just collection if.

How would collection if work with a function such as this?:

void f([int? a, String? b, bool? c]) {
  print('$a, $b, $c');
}

// called like so
f(if (cond1) 5, if (cond2) 'A', if (cond3) true);

I assume the parameters would be assigned null when the corresponding condition is false, but that behavior is different from collection if which does not insert null values into a collection when the condition is false but rather omits the value altogether. Or in other words, if you had the same exact three collection if statements in a list literal, and all three conditions were false the result would be [] not [null, null, null].

@lrhn lrhn changed the title Allow collection if in parameter lists Allow collection if in argument lists Aug 31, 2023
@lrhn
Copy link
Member

lrhn commented Aug 31, 2023

I'm starting to lean very strongly towards this feature, a collection-if like absence of a value, as the preferred way to handle passing or not passing arguments.

We can use else default as the syntax, but that (edit for precision: "that" being adding the keyword default as an argument element in the grammar below, representing no argument value, and not any other hypothetical use of the word default), doesn't actually buy us anything. Being allowed to have no else branch is just as functional, and shorter. (Also ?null would do the same thing and it's two characters shorter.)

(A way to extract the default value of a function parameter, as an expressible value, simply doesn't work. You don't actually know that the default value has a type that is assignable to the static type of the function's parameter, as you know it.)

So, proposal:

Design

We introduce a conditional argument, using a collection-if like syntax.

Example:

foo(if (v != null) v);     // Positional argument
bar(baz: if (v != null) v); // Named argument

These calls will either pass a non-null value as first positional/baz-named argument, or it will not pass a value for that argument.

(We also want to introduce a null-aware element, ?e, which evalautes e to a value, and then omits the value if it's null. That should work both in collections and in arguments. That will behave similarly to if (v != null) v, where v is the value of the expression after ?, which is, as usual, only evaluated once.)

If an argument-element doesn't evaluate to a value, either because its ?e and e evaluated to null, it's if (c) e with no else and c evaluated to false, or c evaluated to true and e doesn't evaluate to a value, then the argument value for that position is elided. The position is always known.

An argument value being elided differs slightly from having no argument.
A foo(if (v != null) v) won't be allowed unless foo can accept one argument with type NonNull(typeof(v)).

Also, calls like foo(1, if (b) 2, 3); may elide the second positional argument, but definitely pass a third.
It won't remove the second argument, and move 3 up to be the second positional argument instead, that would be impossible to type. It just elides a value for the second positional argument, and still passes a value for the third, which isn't currently possible.

Because of that, we might as well allow unconditionally eliding positional arguments,
which we do by just omitting everything before the next comma: foo(1, , 3).
(We don't allow that for named arguments, because that would still require you to write the name,
bar(1, foo:, 3), but since it has no effect on the position of later arguments, you can just not write the foo:.)

A foo(,,) call will only be allowed if foo has two optional leading positional parameters. It elides the argument values, but it insists that there is an optional parameter to elide it for.

Generally, you can only elide values for optional parameters.

You can elide arguments in dynamic invocations, in which case the argument list may have holes,
and it will be a runtime error if the function being called doesn't have an optional parameter at that position.

Grammar

The current argument grammar,

<arguments> ::= `(' (<argumentList> `,'?)? `)'

<argumentList> ::= <argument> (`,' <argument>)*

<argument> ::= <label>? <expression>

becomes

<arguments> ::= `(' <argumentList> `)'

<argumentList> ::= (<argument>? `,')* <argument>?

<argument> ::= <identifier> `:' <argumentElement> | <argumentElement>

<argumentElement> ::= 
      <expression> 
    |`if' `(' <expression> `)' <argumentElement> (`else' <argumentElement>)?
    | `?` <expression>

The <argumentList> is restructured compared to the original, because we want the grammar to distinguish the <argument>? in (<argument>? `,')*, which represents an elided argument value, from the trailing <argument>? which represents no argument at all if omitted. Also, because we now allow an argument list of the form (,), which is an argument list with one elided positional argument, and a trailing comma.

Static semantics

We infer types for each argument position as usual. If there is a corresponding parameter type in the static function type being invoked, we use that parameter type as context type for the argumentElement.

The static type of an <argumentElement> is either a type, T, if the element is unconditional and cannot elide a value, or it's a lifted type T if the value is potentially elided.

The static type of an <argument> is the unlifted type.

Type inference of <argumentList> with a context parameter list signature scheme C, proceeds as follows:

  • Start with an empty argument list type, L = ().
  • For each (<argument>? ,')`:
    • if the <argument> is omitted,
      • let p be 1 + the number of positional arguments in L
      • If C is not _, it's a compile-time error if C does not have a pth position parameter,
        or if that parameter is not optional.
      • let T be Never.
    • Otherwise
      • if <argument> is <identifier> :' `
        • let p be the identifier and e be the element.
        • If C is not _,
          • it's a compile-time error if C does not have a named parameter named id,
          • Let C2 be the type scheme of the named parameter p in C.
          • let cond be true if the named parameter p in C is optional, and false otherwise.
        • otherwise, C is _ (no context type help, a dynamic invocation)
          • let C2 be _.
          • let cond be true.
      • Otherwise <argument> is an <argumentElement> e.
        • let p be 1 + the number of positional arguments in L
        • If C is not _,
          • it's a compile-time error if C does not have a pth position parameter,
          • Let C2 be the type scheme of the pth positional parameter in C.
          • let cond be true if pth positional parameter in C is optional, and false otherwise.
        • otherwise, C is _ (no context type help, a dynamic invocation)
          • let C2 be _.
          • let cond be true.
      • Then perform element types inference on the element e with typing context C2 (see below)
      • Let S be the static type of e.
        • If S is R, then
          • It's a compile-time error if cond is false.
          • Otherwise let T be R.
        • Otherwise let T be S.
  • If p is an integer, append T as pth positional parameter type to L.
  • otherwise p is an identifier, then add a named parameter type p : T to L

where we perform element type inference on an <argumentElement> e with typing context C as follows:

  • If e is an <expression>:
    • perform type inference on e with typing context C.
    • If S is the static type of the expression, S is the static type of the element as well.
  • If e is ?e2, where e2 is an `.
    • If C is _, let C2 be _, otherwise let C2 be C?.
    • Perform type inference on e2 with typing context C2.
    • If the static type of e2 is S then:
      • If S is is non-nullable (<: Object), the static type of e is S.
      • Otherwise the static type of e is NonNull(S).
  • If e is if (e2) e3 where e2 is an <expression> and e3 is an <argumentElement>.
    • Perform type inference on e2 with typing context bool.
    • It's a compile-time error if the static type of e2 is not assignable to bool.
    • Perform element type inference on e3 with typing context C, let S be the static type of e3.
    • If S is R, then the static type of e is S
    • Otherwise the static type of e is S.
  • If e is if (e2) e3 else e4 where e2 is an <expression> and e3 and e4 are <argumentElement>s.
    • Perform type inference on e2 with typing context bool.
    • It's a compile-time error if the static type of e2 is not assignable to bool.
    • Perform element type inference on e3 with typing context C, let S1 be the static type of e3.
    • Perform element type inference on e4 with typing context C, let S2 be the static type of e4.
    • The static type of e is UP(S1, S2), defined as follows:
      • UP(S1, S2) = UP(S1,S2)
      • UP(S1, S2) = UP(S1,S2)
      • UP(S1, S2) = UP(S1,S2)
      • UP(S1, S2) = UP(S1,S2)

Effectively we introduce a new kind of bottom-type, representing no value instead of non-termination.
(The one-branch if behavior is equivalent treating the missing branch as having static type Never.)

Runtime semantics

An runtime argument list can now contain elided values, which the implementations can implement however they want. Calling with an elided value triggers a default value, exactly as passing no argument at all.
The only difference is that positional optional parameters can now get their default value, even if later (necessarily also optional) positional parameters do not.

A dynamic invocation will throw an error if an elided value is passed to a non-optional parameter, or if the function does not have any parameter at that position.
(Optional, allow runtimes to drop named and trailing positional elided values before trying to call..)

A noSuchMethod invocation triggered for a call with elided values, must replace non-trailing eluded values with null in the reified Invocation.positionalArguments. Elided trailing positional arguments and elided named arguments must be omitted from the positionalArguments and namedArguments collections.

Later work

If it's possible to control the arguments in a function call to this degree,
then it's also viable to allow the function to recognized whether an argument was passed or not.

Without the ability to easily control whether an argument is passed or not, forwarding arguments can be prohibitive if the called function can check whether each argument was passed or not.
It may require an exponential number of distinct call expressions to cover all the possible ways a function can be called. With argument-elements, it can be done in one call expression.

Possible approaches could be:

  • Allow non-constant default value expressions. (Those can have side-effects which allows a caller to recognize whether that expression was evaluated or not).
  • Allow declaring an optional parameter as late, which makes it not have a value at all, if none was passed. Then introduce a way to query a late variable for whether it has a value.

@eernstg
Copy link
Member

eernstg commented Sep 1, 2023

So, proposal:

Interesting! I think it's definitely worth considering how we could enable provision/non-provision of arguments to optional parameters in a programmatic manner. On the other hand, this idea does seem to impose a non-trivial amount of complexity on the meaning of call sites.

We can use else default as the syntax, but that doesn't actually buy us anything. Being allowed to have no else branch is just as functional, and shorter.

Well, if we have support for default (using something like #2269) then we could also use else default + 1 or any other expression where default denotes the relevant default value. Another example arises if we wish to have a different signature in a nearly-forwarding function:

// This is what we have.
void f([int x = 42, int y = 314]) {...}

// But someone wants a callback where the parameters are swapped.
void g([int y = f.default[1], int x = f.default[0]]) => f(x, y);

// Or they want to use named parameters.
void h({int x = f.default[0], int y = f.default[1]}) => f(x, y);

So I don't think it's fair to say that it doesn't buy us anything to use an explicit denotation of a default value.

(A way to extract the default value of a function parameter, as an expressible value, simply doesn't work. You don't actually know that the default value has a type that is assignable to the static type of the function's parameter, as you know it.)

I commented on this topic here, which seems to be a more relevant context.

@lrhn
Copy link
Member

lrhn commented Sep 4, 2023

Allowing a sole default in tail position to represent passing no value buys us no value over just omitting an else branch (and with null-aware arguments, ?null is the same as a tail-aware default).

Allowing a default-operator to read default values of arbitrary parameters, into positions that are not just arguments to that parameter, is a much different feature, and one which I'm quite opposed to. Because I consider a parameter default value to be an implementation detail, and accessing it from outside the function breaks encapsulation of hat implementation detail.
If we get more ways to specify default values, perhaps including non-constant ones, it clearly has to be an implementation detail what it does, not part of your public API. The fact that default values are currently constant only is a red herring. We should assume that they are not, and there is no way we'd allow a non-constant default value expression to be triggered from outside of its scope.

@eernstg
Copy link
Member

eernstg commented Sep 4, 2023

Of course, it isn't difficult to reach the conclusion that "this approach doesn't contribute anything of value" if you start by implicitly redefining "this approach" in such a way that it contributes absolutely nothing.

Allowing a default-operator to read default values of arbitrary parameters, into positions that are not just arguments to that parameter, is a much different feature

No, that's exactly the kind of feature that I proposed in dart-lang/sdk#58240 and referred to several times in this issue, starting here. This confirms very directly that you are using a straw man argument.

Never mind, here's a real argument:

I consider a parameter default value to be an implementation detail

There could be several reasons why that perspective is suboptimal: A default value which is declared in the way we use today, considered as part of the API, ...

  • is machine-checked, which is usually considered to be a good property. For example, we generally consider machine-checked types to be helpful: List<String> strings is better than dynamic strings; // A list of strings. because we get help from tools to achieve strict enforcement). In that sense, an explicitly declared default value is safer than having a DartDoc that contains "If you omit p then it works like passing thatDefaultValue" (which may or may not be the same expression as the one which is actually used as the default value, no matter where we do that syntactically).
  • occurs syntactically in the header of the function or method declaration (I know, the 'header' is an informal term, but we could easily define it, and surely we all know pretty well what it means). As such, it is naturally a part of the public interface of the function or method.
  • is relevant to callers: Whenever the given function or method is called, and no actual argument is provided for an optional parameter with a default value, the author/maintainers of the call site need to understand the effect of omitting that particular actual argument. In other words, the default value plays a public role.

It is of course highly error prone to declare a default value for a parameter in an instance method declaration, and then declare a different default value for the "same" parameter in an overriding declaration. I created dart-lang/sdk#59293 in order to maintain that there should be support for detecting this situation, and acting on it.

If we get more ways to specify default values, perhaps including non-constant ones, it clearly has to be an implementation detail what it does,

It clearly has to be a well-documented property of an instance method declaration what the semantics of omitting a given actual argument is. Saying "you can pass this argument or omit it, but it is none of your business what happens if you omit it" is not very helpful.

Sure, you can write a long story about said semantics in a DartDoc comment. I'm just noting that a plain expression in the declaration of the optional parameter may be a useful, readable, and concise way to document this behavior.

The fact that it is possible to write error-prone code where a given default value is overridden by a different value is unfortunate. But (1) it's easy to fix, just enable that lint and correct any discrepancies, and (2) the approach where the default value is considered to be an implementation detail does not eliminate this problem, it just turns the problem into a mistake that we won't get any help at all to avoid.

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

No branches or pull requests

5 participants