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

Record spreading #2128

Open
lrhn opened this issue Feb 23, 2022 · 8 comments
Open

Record spreading #2128

lrhn opened this issue Feb 23, 2022 · 8 comments
Labels
patterns Issues related to pattern matching.

Comments

@lrhn
Copy link
Member

lrhn commented Feb 23, 2022

The current records proposal does not introduce a "spreading" operator.
I think it should have one.

A "record spread", with syntax ... recordExpression, would be allowed inside any record expression or argument list.
The recordExpression must have a static type which is an actual record type (not dynamic and not Record or T extends Record).

It works by, effectively, inlining every member of the tuple at the spread-point. So,

(num, num) point = ...;
(num, num, {Color color}) colorPoint = (...point, color: Color.red};

would inline the elements of point as elements of the new record at the point of the spread.

It would also work in argument lists.

This is a static operation which requires knowing the type of the spreadee and allows knowing the structure of the result.
It won't allow something like

Record concat(Record a, Record b) => (...a, ...b); // Not allowed!

because neither a nor b have known structures.

That also means that it can be desugared into, e.g, (point[0], point[1], color: Color.red) if we have a syntax for directly accessing members of a record. (If we don't, the desugaring would include a pattern match of some sort, and might not be directly expressible as an expression in the language.)

@lrhn lrhn added the patterns Issues related to pattern matching. label Feb 23, 2022
@munificent
Copy link
Member

I think it should have one.

Me too!

The recordExpression must have a static type which is an actual record type (not dynamic and not Record or T extends Record).

Assuming we stick with the idea to use Destructure_n_ interfaces for positional destructuring, it might be reasonable to also allow types that implement Destructure_n_ for some n. (Though that raises questions of which n if it implements multiple...)

It would also work in argument lists.

YES. [jack nicholson nodding.gif]

Also, we should allow if in record literals and argument lists. And potentially even for for positional fields and arguments.

@lrhn
Copy link
Member Author

lrhn commented Feb 24, 2022

I really do not want the DestructureN interfaces.
Implementing a Destructure4 interface suggests that there is an inherent and natural view of the class as a four-tuple (and not with any named record entries), but then, maybe just make the class a "view" on a four-tuple to being with.

If you want to represent your class as a tuple, I'd just have a method or getter returning that tuple. That makes it explicit, and documented by name, which semantic view of the object you're using.
If you can't find a good name, then that tuple view might not belong as part of the API anyway.

I'm not sold on if and for in records, if they mean that we cannot statically determine the structure of the record/argument list.
If I write var p = (1, if (b) 2, 3);, what will be the type of p? If it's (int, int?, int), then maybe, but we already have the explicit b ? 2 : null` for that.
For argument lists, omitting an optional argument makes sense, but again, it should not change the position of any later argument. Repetition does not make sense in the current argument lists.

The places where it makes sense to use if/for/spread is in arbitrary-length sequences of similarly typed "things", because there it doesn't change the static type to omit or insert more of those things. (Monoids, basically.)
So, if we had functions with rest parameters (arbitrary number of similarly typed arguments collected into a single parameter), then it would make sense to spread/if/for into those.
(Strings with concatenation is a monoid, so string interpolations should allow for/if/spread too, #1478).

@munificent
Copy link
Member

I really do not want the DestructureN interfaces.

I'm not particularly attached to them, but they do seem to solve the problem that I want to solve which is giving classes a way to opt into and control their positional destructuring.

Implementing a Destructure4 interface suggests that there is an inherent and natural view of the class as a four-tuple (and not with any named record entries), but then, maybe just make the class a "view" on a four-tuple to being with.

I think implementing Destructure4 only says that the author of the class has decided that there is a natural way to destructure the class into four positional fields. I think that's directly analogous to a class having an unnamed constructor with four positional arguments. In that case, the author isn't claiming that that there's something deeply intrinsic to that way to construct the instance, but they are saying "Here's the way I've decided it can be constructed from four positional arguments."

Implementing Destructure4 just says, "Here's the way I've decided it can be destructured to four positional subpatterns."

I'm not sold on if and for in records, if they mean that we cannot statically determine the structure of the record/argument list.
If I write var p = (1, if (b) 2, 3);, what will be the type of p? If it's (int, int?, int), then maybe, but we already have the explicit b ? 2 : null` for that.

Good point.

The places where it makes sense to use if/for/spread is in arbitrary-length sequences of similarly typed "things", because there it doesn't change the static type to omit or insert more of those things. (Monoids, basically.)
So, if we had functions with rest parameters (arbitrary number of similarly typed arguments collected into a single parameter), then it would make sense to spread/if/for into those.

Yes, rest params was where I initially thought to support if and for in argument lists and is, I think, still the place where they make the most sense.

@ds84182
Copy link

ds84182 commented Mar 9, 2022

Have you also considered introducing support to spread record typedefs in parameter lists as well?

Example of how this can greatly improve passing parameters to extended/nested widgets:

typedef CommonButton = ({
  VoidCallback? onPressed,
  VoidCallback? onLongPress,
  ValueChanged<bool>? onHover,
  ValueChanged<bool>? onFocusChange,
  ButtonStyle? style,
  FocusNode? focusNode,
  bool autofocus = false,
  required Widget child,
});

class OutlinedButton extends ButtonStyleButton {
  // Only for OutlinedButton!
  final bool someSpecializedThing;

  OutlineButton(CommonButton... commonButton, {this.someSpecializedThing}) : super(...commonButton);
}

// Not exactly a clean example, but shows what could be possible:
class MyParticleButton extends StatelessWidget {
  final CommonButton commonButton;
  // Also works for function types
  final Widget Function(CommonButton...) builder;
  final Color particleColor;

  MyParticleButton(CommonButton... commonButton, {Key? key, this.builder = OutlinedButton.new, this.particleColor}) : super(key: key);

  Widget build(BuildContext context) {
    return ParticlePainter(
      child: builder(
        ...commonButton,
        // Like map spreads, allows overwriting positional parameters?
        onPressed: ParticlePainter.wrap(context, commonButton.onPressed),
      ),
    );
  }
}

// Usage:
MyParticleButton(child: Text('Tap me!'))
MyParticleButton(builder: ElevatedButton.new, child: Text('I am filled with color (and particles)!'))

@Wdestroier
Copy link

Record concat(Record a, Record b) => (...a, ...b); // Not allowed!

Shouldn't it be possible to spread records even if they're represented by a dynamic type?
concat(a, b) => (...a, ...b);

@lrhn
Copy link
Member Author

lrhn commented Mar 14, 2022

Whether we can allow dynamic record operations depends on how we want to be able to implement records.

There are implementation strategies (that I favor) which requires all record types occurring in the program to be known at compile-time.
I guess that we can do arbitrary (and slow and expensive) record operations like the one above, as long as the result is always boxed, but even the boxing would the need to know and record the structure in a different way than optimized (compile-time known) records, which will make boxed-record access polymorphic and slower.

So, that's why I don't want to allow you to do any operation on an untyped record, other than checking that it has a specific known structure.

@Wdestroier
Copy link

Can records be created during runtime?

void handleWebSocket(WebSocket webSocket) {
  webSocket
    .map((string) => decodeJson(string))
    .listen((message) {
      var echo = message.echo;
      print("Message to be echoed: $echo");
      var response = encodeJson((response: echo));
      webSocket.add(response);
    }, onError: (error) {
      print('Bad WebSocket request');
    });
}

The spread operator with dynamic types would be more useful if so. It would allow the creation of something more equivalent to a JSON in Dart than a Map, but I'm not sure if it's of interest of this proposal or even the Dart language. I guess it would be useful especially when interacting with APIs.

@munificent munificent added patterns-later and removed patterns Issues related to pattern matching. labels Sep 13, 2022
@munificent
Copy link
Member

I'm moving this over to patterns-later. I definitely want it, but I think it would be really hard to get this designed and into the initial release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patterns Issues related to pattern matching.
Projects
None yet
Development

No branches or pull requests

4 participants