www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Feedback Thread: DIP 1041--Attributes for Higher-Order

reply Mike Parker <aldacron gmail.com> writes:


This is the feedback thread for the first round of Community 
Review of DIP 1041, "Attributes for Higher-Order Functions".

**THIS IS NOT A DISCUSSION THREAD**

I will be deleting posts that do not follow the Feedback Thread 
rules outlined at the following link:

https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md

The rules are also repeated below. Recently, I have avoided 
deleting posts that violate the rules if they still offer 
feedback, but I'm going to tighten things up again. **Please 
adhere to the feedback thread rules.**

The place for freeform discussion is in the **Discussion Thread** 
at:

https://forum.dlang.org/post/lqjpyhkfnorfiflrflct forum.dlang.org

You can find DIP 1041 here:

https://github.com/dlang/DIPs/blob/11fcb0f79ce7ec209fb2a302d1371722d0c8ad82/DIPs/DIP1041.md

The review period will end at 11:59 PM ET on April 26, or when I 
make a post declaring it complete. At that point, this thread 
will be considered closed and any subsequent feedback may be 
ignored at the DIP author's discretion.


Posts in this thread that do not adhere to the following rules 
will be deleted at the DIP author's discretion:

* All posts must be a direct reply to the DIP manager's initial 
post, with the following exceptions:
     - Any commenter may reply to their own posts to retract 
feedback contained in the original post;
     - The DIP author may (and is encouraged to) reply to any 
feedback solely to acknowledge the feedback with agreement or 
disagreement (preferably with supporting reasons in the latter 
case);
     - If the DIP author requests clarification on any specific 
feedback, the original commenter may reply with the extra 
information, and the DIP author may in turn reply as above.
* Feedback must be actionable, i.e., there must be some action 
the DIP author can choose to take in response to the feedback, 
such as changing details, adding new information, or even 
retracting the proposal.
* Feedback related to the merits of the proposal rather than to 
the contents of the DIP (e.g., "I'm against this DIP.") is 
allowed in Community Review (not Final Review), but must be 
backed by supporting arguments (e.g., "I'm against this DIP 
because..."). The supporting arguments must be reasonable. 
Obviously frivolous arguments waste everyone's time.
* Feedback should be clear and concise, preferably listed as 
bullet points (those who take the time to do an in-depth review 
and provide feedback in the form of answers to the questions in 
the documentation linked above will receive much gratitude). 
Information irrelevant to the DIP or which is not provided in 
service of clarifying the feedback is unwelcome.
Apr 12 2021
next sibling parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 12.04.21 11:38, Mike Parker wrote:


 
 ...
 
 You can find DIP 1041 here:
 
 https://github.com/dlang/DIPs/blob/11fcb0f79ce7ec209fb2a302d1371722d0c8
d82/DIPs/DIP1041.md 
 ...


Unfortunately, it is not written too well: The reader gets flooded with 
details way before being told what the problem actually is or how the 
proposal addresses it.

As far as I can tell, this is trying to introduce attribute polymorphism 
without actually adding polymorphism, much like `inout` attempted and 
ultimately failed to do. I am very skeptical. It's taking a simple 
problem with a simple solution and addressing it using an overengineered 
non-orthogonal mess in the hopes of not having to add additional syntax.

To add insult to injury, the first example that's shown in the DIP as 
motivation abuses an existing type system hole. `toString` is `const`, 
`sink` is `const`, the only reference to result accessible to `toString` 
is in the context of `sink`, but somehow `result` is mutated anyway. 
Unsoundness should be fixed, not embraced!

Finally, there's this concern: The DIP assumes that the only reasonable 
way to manipulate delegates in higher-order functions involves calling 
them, but this is not accurate. E.g.:

---
auto compose(A,B,C)(C delegate(B) f, B delegate(A) g)pure{
     return a=>f(g(a));
}
---

With the proposed changes, composing impure functions suddenly becomes 
an impure operation as soon as you abstract it into a higher-order 
function. This is pure nonsense. If you have a `pure` expression and 
abstract it into a `pure` function, it should not become less `pure` in 
the process!


E.g., consider this:

---
auto compose2(A,B,C)(C delegate(B) f, B delegate(A) g)pure{
     return compose(f,g);
}
---

With the changes proposed in the DIP, this does not even compile, 
breaking abstraction/perfect forwarding.
Apr 12 2021
parent Q. Schroll <qs.il.paperinik gmail.com> writes:
On Monday, 12 April 2021 at 11:05:14 UTC, Timon Gehr wrote:

 On 12.04.21 11:38, Mike Parker wrote:

 https://github.com/dlang/DIPs/blob/11fcb0f79ce7ec209fb2a302d1371722d0c8
d82/DIPs/DIP1041.md ...


 Unfortunately, it is not written too well: The reader gets 
 flooded with details way before being told what the problem 
 actually is or how the proposal addresses it.


Doesn't the Abstract explain what the problem is and give a 
general idea how it is addressed?


 As far as I can tell, this is trying to introduce attribute 
 polymorphism without actually adding polymorphism, much like 
 `inout` attempted and ultimately failed to do. I am very 
 skeptical. It's taking a simple problem with a simple solution 
 and addressing it using an overengineered non-orthogonal mess 
 in the hopes of not having to add additional syntax.


You're mistaken. You can take a look at the Alternatives for 
seemingly simple solutions. There ain't any. Because D isn't an 
immutable-all-the-way-down language like e.g. Haskell, none of 
the easy solutions are sound. You always have the problem of 
assigning the parameter in the functional unless it's `const` or 
another flavor of non-mutable.
If you don't go the `const` route, you have to deal with 
assignments to the parameter before it's called. You have to 
disallow assignments that, looking at the types, are a 1-to-1 
assignment. IMO, going via `const` is far more intuitive.

In fact, "not having to add additional syntax" was never the 
motivation for the proposal. Not having to introduce attributes 
_specific_ to higher-order function was.


 To add insult to injury, the first example that's shown in the 
 DIP as motivation abuses an existing type system hole.


I disagree that it is a hole in the type system. When having 
`qual₁(R delegate(Ps) qual₂)` where `qual₁` and `qual₂` are type 
qualifiers (`const`, `immutable`, etc.) it is practically most 
useful if `qual₁` only applies to the function pointer and (the 
outermost layer of) the context pointer while `qual₂` refers to 
the property of the context itself.
Since the language gives no non-UB way to assign the function 
pointer and the context pointer separately, it is not unsound.
[Here](https://forum.dlang.org/post/gauloixsonnnlswhbiqe forum.dlang.org) is
the space for discussing this issue. Unfortunately, it's mostly us two that
care.


 `toString` is `const`, `sink` is `const`, the only reference to 
 result accessible to `toString` is in the context of `sink`, 
 but somehow `result` is mutated anyway.


See the paragraph above.


 Unsoundness should be fixed, not embraced!


Yes, I guess no one disagrees on that one, but on the question of 
it being an instance of it.


 Finally, there's this concern: The DIP assumes that the only 
 reasonable way to manipulate delegates in higher-order 
 functions involves calling them, but this is not accurate.


It assumes that the the most common use-case of non-mutable 
delegate parameters is only calling them. Returning them is 
another, but a rarer one. The DIP details that in this case, the 
author of the `compose` function needs to remember not to make 
the parameters mutable.


 ```D
 auto compose(A,B,C)(C delegate(B) f, B delegate(A) g)pure{
     return a=>f(g(a));
 }
 ```

 With the proposed changes, composing impure functions suddenly 
 becomes an impure operation as soon as you abstract it into a 
 higher-order function. This is pure nonsense. If you have a 
 `pure` expression and abstract it into a `pure` function, it 
 should not become less `pure` in the process!


You did it correctly in the sense of the DIP. `compose` takes `f` 
and `g` as mutable. None of the proposed changes apply to mutable 
delegate parameters. By the changes proposed by this DIP, 
`compose` is `pure`. However, all delegates you pass to it lose 
information of attributes because you _could_ assign `f` or `g` 
in `compose`, no problem.

But as you don't intend to mutate `f` or `g` in it, you could get 
the idea of making them `const` like this:
```D
C delegate(A) compose(A, B, C)(const C delegate(B) f, const B 
delegate(A) g) pure
{
     return a => f(g(a));
}
```
Then, by the proposed changes, only `pure` arguments lead to a 
`pure` call expression.
However, `compose` is a good example why this is not an issue: It 
is already a template. Why not go the full route and make the 
`delegate` part of the template type arguments like this:
```D
auto compose(F : C delegate(B), G : B delegate(A), A, B, C)(F f, 
G g) pure
{
     return delegate C(A arg) => f(g(arg));
}
```
Unfortunately (see 
[here](https://issues.dlang.org/show_bug.cgi?id=21823)), when 
calling `compose` with a `const` declared delegate value, D's 
IFTI infers `const` for the parameter type although the parameter 
is copied. I didn't realize that when writing the DIP. This is a 
problem, but it can be fixed (probably should). To be honest, it 
was my C++ background that tricked me into thinking copying stuff 
removes `const` in any case.

It's not without solution, however, the solution being a cast or 
explicit template instantiation, none of which is nice:
```D
const dgAtoB = ...;
const dgBtoC = ...;
auto dgAtoC1 = compose!(C delegate(B), B delegate(A))(dgBtoC, 
dgAtoB); // no IFTI, no cast
auto dgAtoC2 = compose(cast() dgBtoC, cast() dgAtoB); // IFTI 
with cast
```
The cast is ` safe`.


 E.g., consider this:

 ```D
 auto compose2(A,B,C)(C delegate(B) f, B delegate(A) g)pure{
     return compose(f,g);
 }
 ```

 With the changes proposed in the DIP, this does not even 
 compile, breaking abstraction/perfect forwarding.


No, because with mutable parameters, nothing changes. The DIP is 
_very_ clear about that.
Apr 12 2021
prev sibling next sibling parent reply Max Haughton <maxhaton gmail.com> writes:
On Monday, 12 April 2021 at 09:38:10 UTC, Mike Parker wrote:



 This is the feedback thread for the first round of Community 
 Review of DIP 1041, "Attributes for Higher-Order Functions".

 [...]


I'm still processing this DIP so substantive critique will arrive 
later, but the first thing that comes to mind is that this DIP 
really really needs to go on a diet - compartmentalize the 
changes into a section *then* rationalize them.
Apr 12 2021
parent Q. Schroll <qs.il.paperinik gmail.com> writes:
On Monday, 12 April 2021 at 15:28:16 UTC, Max Haughton wrote:

 On Monday, 12 April 2021 at 09:38:10 UTC, Mike Parker wrote:



 This is the feedback thread for the first round of Community 
 Review of DIP 1041, "Attributes for Higher-Order Functions".

 [...]


 I'm still processing this DIP so substantive critique will 
 arrive later, but the first thing that comes to mind is that 
 this DIP really really needs to go on a diet - compartmentalize 
 the changes into a section *then* rationalize them.


If you can, tell me which parts you deem unnecessary (answer to 
this message). I know that the text is long. The format of a DIP 
as I understood it (Rationale first, Description second, required 
to be separate sections) doesn't really allow section-wise 
description and rationalization.
Apr 12 2021
prev sibling parent Mike Parker <aldacron gmail.com> writes:
On Monday, 12 April 2021 at 09:38:10 UTC, Mike Parker wrote:


 The review period will end at 11:59 PM ET on April 26, or when 
 I make a post declaring it complete. At that point, this thread 
 will be considered closed and any subsequent feedback may be 
 ignored at the DIP author's discretion.


The review period has ended. Thanks to everyone who left feedback.
Apr 27 2021