[RFC] Fix Issue 22736 - Add destructuring bind for std.typecons.Tuple tuples

[CyberShadow]

References:

• destructuring-bind in Common Lisp (though here we only do flat tuples)
• Destructuring assignment in ES6
  • Destructuring objects as function parameters in ES6 (except we do this with arrays, not objects)
• t.bind!fun is equivalent to t.expand.Identity!fun

Implementation TODO:

• Avoid copying when possible
• Reject tuples with named members (to perhaps allow a future implementation to perform name-based structuring, like for ES objects)

[dlang-bot]

Thanks for your pull request, @CyberShadow!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	22736	enhancement	Add destructuring bind for std.typecons.Tuple tuples

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#8372"

[RazvanN7]

I will defer the acceptance of this to @atilaneves

[pbackus]

I have a more general implementation of this in my personal "utilities" module that

• Works with all struct types (via .tupleof), not just Tuple.
• Makes no unnecessary copies (and therefore can handle non-copyable types).
• Is "curried" to allow writing code like alias g = bind!f without the use of std.meta.ApplyLeft (or equivalent).

I currently use the name apply for this template, also based on Lisp, although I think bind is fine too (I originally called it unpack).

Here's a gist with the code for my version: https://gist.github.com/pbackus/90c333fb17422e1e8477ab68214dbb3a

[CyberShadow]

That looks nice.

• Works with all struct types (via .tupleof), not just Tuple.

Structs' fields generally have names, so I think having it for tuples makes for a more compelling use case.

• Is "curried" to allow writing code like alias g = bind!f without the use of std.meta.ApplyLeft (or equivalent).

Unfortunately this does not allow restricting the template to Tuple, which will probably be unreasonably disruptive for such an addition.

The reason this is a top-level function and not a Tuple method is the multiple-context limitation of methods taking predicates via lambda template parameters.

I currently use the name apply for this template, also based on Lisp, although I think bind is fine too (I originally called it unpack).

I've been tempted to call so many things apply that I think it's perhaps wisest to not use this name at all :)

[pbackus]

Structs' fields generally have names, so I think having it for tuples makes for a more compelling use case.

Both struct fields and tuple fields can be either named or (individually) anonymous. I don't see any reason to give std.typecons.Tuple in particular special treatment here.

Unfortunately this does not allow restricting the template to Tuple, which will probably be unreasonably disruptive for such an addition.

Yes it does. You just put isTuple!T where my version has is(T == struct). Although I still can't see any good reason for such a restriction.

The reason this is a top-level function and not a Tuple method is the multiple-context limitation of methods taking predicates via lambda template parameters.

I'm familiar with the technique.

I've been tempted to call so many things apply that I think it's perhaps wisest to not use this name at all :)

You're probably right. It's not a very suggestive name on its own if you don't get the Lisp reference.

[CyberShadow]

Both struct fields and tuple fields can be either named or (individually) anonymous. I don't see any reason to give std.typecons.Tuple in particular special treatment here.

Because it has a much stronger use case, we can be more comfortable in claiming the word bind for this particular purpose (std.typecons tuples). Claiming it for all structs would mean we would have to consider the broader implications of how this term is used throughout the D community, and it would also probably have to be placed somewhere else, not std.typecons.

Yes it does. You just put isTuple!T where my version has is(T == struct).

No, it doesn't work.

https://gist.github.com/CyberShadow/f099037e99b796d5058d2ca3fdb4078f

test.d(6): Error: template `b.bind(int i)` at b.d(3) conflicts with template `a.bind(int i)` at a.d(3)

You're probably right. It's not a very suggestive name on its own if you don't get the Lisp reference.

There's also other programming languages and probably other professional areas where such terms have specific meanings, so we should be considerate of those with other experiences.

[pbackus]

Because it has a much stronger use case, we can be more comfortable in claiming the word bind for this particular purpose (std.typecons tuples). Claiming it for all structs would mean we would have to consider the broader implications of how this term is used throughout the D community, and it would also probably have to be placed somewhere else, not std.typecons.

I'd argue that we should consider the implications of the name either way, but it is true that the cost of a mistake is a little lower for the less-general version.

I agree that it would have live somewhere other than std.typecons—probably std.functional.

https://gist.github.com/CyberShadow/f099037e99b796d5058d2ca3fdb4078f

In general, if you import symbols with the same name from two different modules, you may have to disambiguate. Fortunately, the language gives you plenty of tools to do so (qualified names, aliases, static import).

In practice, there are no other symbols in Phobos that std.typecons.bind or std.functional.bind would conflict with, so the problem is purely hypothetical. I'm not convinced that it's worth giving up on useful functionality to solve a hypothetical problem that the language already provides solutions for.

Experience with std.algorithm.iteration.map and other templates of its ilk also suggests that this is not likely to be a big deal in practice.

[pbackus]

I should mention that the strongest motivation for the "curried" version is use in range pipelines:

someRange
    .enumerate
    .map!(bind!((index, element) => doSomethingWith(index, element)))
    .each!doSomethingElse;

With this PR's current implementation, the above usage is impossible, and one instead has to write

    .map!(tup => tup.bind!((index, element) => doSomethingWith(index, element)))

[CyberShadow]

With this PR's current implementation, the above usage is impossible, and one instead has to write

IMHO, it seems less annoying to add t => t. than deal with import conflicts. It is also more self-describing and is closer to our intent, because the only reason bind is not a Tuple method in this pull request is due to a technical limitation of the language's implementations today.

Also, I would recommend to stay away from destructuring tuples of named items into a list, because Tuple!("latitude", float, "longitude", float)(latitude, longitude).bind!((longitude, latitude) => ...) would be wrong despite the semantic equivalent being correct in JavaScript.

[pbackus]

IMHO, it seems less annoying to add t => t. than deal with import conflicts.

If this is added as-is, there is a 100% chance I will be forced to endure the annoyance of writing t => t. every time I use it in a range pipeline. If the curried version is added, there is a much, much lower chance that I will have to deal with an import conflict for any given usage. Therefore, I conclude that the expected annoyance of the current implementation is higher than that of the curried implementation.

It is also more self-describing and is closer to our intent, because the only reason bind is not a Tuple method in this pull request is due to a technical limitation of the language's implementations today.

My hope here is that your (and Atila's) intent is not set in stone. :)

Also, I would recommend to stay away from destructuring tuples of named items into a list, because Tuple!("latitude", latitude, "longitude", longitude).bind!((longitude, latitude) => ...) would be wrong despite the semantic equivalent being correct in JavaScript.

The same criticism could be leveled at literally any language feature that allows the programmer to bind names to values. I don't think trying to prevent this kind of mistake is (or should be) within bind's jurisdiction.

In your specific example, latitude and longitude are distinct types, so the programmer should get a type error if they attempt to use one in place of the other. IMHO this is a much better way to guard against naming mistakes, since it works not only with bind but with any function the programmer might pass a latitude or longitude to, without requiring any special effort from those functions.

[CyberShadow]

The same criticism could be leveled at literally any language feature that allows the programmer to bind names to values.

Fortunately, we are moving away from such dangerous features; for function arguments, we've had Flag for a while, and now named arguments will solve the main instance more generally.

In your specific example, latitude and longitude are distinct types

Sorry, I messed it up. D's typing is pretty lax anyway, what with implicit int/bool conversions etc.

since it works not only with bind but with any function the programmer might pass a latitude or longitude to

I think the point is to not have to put types in every lambda's parameter list.

[pbackus]

As far as I know, there are no plans to deprecate the following code, which has exactly the same issues as your bind example:

auto t = getCoordinates();
auto longitude = t.expand[0];
auto latitude = t.expand[1];

The fact is, no matter what we do in bind, it is trivially easy to come up with ways that the elements of a tuple could inadvertently get bound to misleading names. So any solution we come up with for bind is, inevitably, only going to address a tiny subset of this class of mistake. To me, that's a strong signal that bind is not the correct place to address this problem.

It's also important to consider what kind of precedent would be set by adding a special case like this to bind. How many future additions to Phobos will also need to be burdened with "anti-renaming" special cases, for the sake of consistency? As Andrei says, "Good Work begets more Good Work."

[CyberShadow]

As far as I know, there are no plans to deprecate the following code

Yes, but why in the world would you write it like that? That's downright maliciously obfuscating the intent, when the tuple already has perfectly good names you could use. It wouldn't pass code review, while the .bind!((longitude, latitude) => ...) plausibly would.

How many future additions to Phobos will also need to be burdened with "anti-renaming" special cases, for the sake of consistency? As Andrei says, "Good Work begets more Good Work."

Rejecting named tuples would already follow a different precedent, in which we allow a new feature for some very narrow subset of parameters first, when it's not clear which semantics to use for the rest. What to do with named tuples can be a separate debate; this pull request is for destructuring tuples with anonymous fields only.

[pbackus]

Rejecting named tuples would already follow a different precedent, in which we allow a new feature for some very narrow subset of parameters first, when it's not clear which semantics to use for the rest.

...and then people complain on the forums that D is full of half-finished features. Maybe if we aren't sure of the correct semantics in all cases, this addition is not quite ready for Phobos yet, and would be better off taking some time to mature on code.dlang.org.

[CyberShadow]

Here is a suggestion:

• Please propose apply / bind for all structs, as implemented in your gist above, for inclusion in Phobos.
• If it gets accepted, this pull request is moot.
• If it gets rejected, we can look at this again.

[atilaneves]

Closed in favour of this PR.