September 11, 2019
On 11/09/2019 5:30 AM, Paul Backus wrote:
> On Tuesday, 10 September 2019 at 09:06:23 UTC, Mike Parker wrote:
>> This is the feedback thread for the second round of Community Review for DIP 1020, "Named Parameters":
>>
>> https://github.com/dlang/DIPs/blob/c723d8f4e3ac2d5bcaf8cdff87e1507f09669ca6/DIPs/DIP1020.md 
>>
> 
> Summary of review: in general, this DIP should do a better job of explaining the reasoning behind its choices of syntax and semantics, especially in cases where clear alternatives were available. Pros and cons should be explicitly discussed, ideally with examples to illustrate them.
> 
> Detailed comments follow.
> 
>> Prior Work
>>
>> Dyalect
> 
> Why does the DIP include some random programmer's personal hobby language alongside ones with significant adoption like C# and Swift? Is it just to give an example of C#'s syntax with a different reordering rule? If so, isn't the fact that you couldn't find a mainstream example of this reordering rule evidence against considering it?

Dyalect from what I understood support full reordering without any restrictions.
It was the first language I could find that had this.

Prior to editing revision 2 included full, partial and no reordering with pros and cons of each as options for voting upon.

It was an example of a language that supported full (most are partial, Swift/Objective-C is the closest to no reordering).

>> Python
>>
>> [...]
>> Parameter syntax: ** Identifier
> 
> This is inaccurate, or at least incomplete. All function parameters in Python can be passed by name; no special parameter syntax is required to enable named arguments. The syntax `** Identifier` (usually written `**kwargs`) is used specifically for *variadic* keyword arguments.

Noted for fixing.

>> Parameters must be marked as accepting named arguments, otherwise there will
>> be no way to determine which overload to resolve.
> 
> The DIP needs to explain why this is true, not just assert it. At the very least, it should give an example of a function call for which overload resolution would fail if named parameters were not explicitly marked.

The previous paragraph describes why you would want this. But I'll take note that it needs improving to make that clearer.

>> It is the author's opinion that the is expression is heavily overloaded and
>> doing too much. It should be possible to access the names of named template
>> parameters outside of an instantiated template, but it should be done with a
>> mechanism other than the is expression.
> 
> Personal opinion is an extremely weak justification, and unlikely to convince anyone who does not already agree with the DIP's author. This section would be much more convincing if it compared and contrasted both approaches, and listed the pros and cons of each.

```d
void func() {
    alias Type = Foo!int;

    static if (is(Type : Foo!T, T)) {
    }
}

struct Foo(Type) {
}
```
	
If the template parameter of Foo was @named, then T in the static if would have to match the name of the parameter.
Which could conflict and the user would be forced to change other parts of the code and not use a name that suits them.
This is a consequence of named parameters being non-positional.

Sadly this change didn't make the cut for this revision.

>> The @named attribute prevents clashes with other existing language features
>> like template specializations (e.g., T:V or T=int). It aims to be opt-in
>> rather than opt-out to prevent clashes with core semantics of the D
>> programming language like function overloading.
> 
> The DIP should give specific examples of these "clashes" and explain how @named prevents them, rather than speaking in vague, general terms.

This is a toughy.

Lets say we have a syntax derived from the argument syntax for parameters:

Type: Expression

becomes

Identifier : TemplateParameter

Which makes this valid:

T::V

but is T:V a named parameter with a name of T with invalid tokens after it or is it a template parameter with a name of T?

I will have to think about how to proceed, but I agree it needs improving.

>> The order of evaluation of named function arguments follows the lexical order
>> of the argument list (full, not subset).
> 
> What does "(full, not subset)" mean? This is the only usage of the word "subset" in the entire DIP. The DIP should define any new terms it intends to use, rather than assume that the reader will infer the correct meaning from context.

I use sublist in the pseudo code of the resolution algorithm and subset here. The order was reversed when it was originally written. Noted.

>> Two new traits are added to support type inspection. The first addition is
>> the isNamedParameter trait, which allows a given parameter from the parameter
>> list of a function, delegate, or a function pointer to be inspected for the
>> presence of the @named attribute. This is intended to be used with the
>> __parameters specialization of the is expression. The second addition is the
>> getNamedParameters trait, which returns a list of all named parameters from
>> the parameter list of a given template, function, delegate, or function
>> pointer.
> 
> Why not have isNamedParameter work for templates too, and get rid of getNamedParameters?

How do you propose to get the names of the template parameters?

Unless there is some trick that I am unaware of that hasn't been added to std.traits, I do not believe that it would work. They are both needed.

>> Overload resolution for symbols (for function and template declarations) is
>> performed without taking named parameters into account. Ignoring named
>> parameters in both cases means that the relevant algorithms and user code
>> does not need to change to accommodate named parameters but they must be
>> modified to filter them out. This is in line with the current behavior. A
>> side effect of this is that name mangling does not need to change to
>> accommodate these new parameters.
>>
>> [...]
>>
>>     void foo(int a) {
>>     }
>>
>>     void foo(int a, @named int b) {
>>     }
>>
>> [...]
>>
>>     foo(1); // error: matches both declarations
>>     foo(1, b: 2); // error: matches both declarations
> 
> Given that the function calls are unambiguous as written, it seems very strange for the compiler to reject them. The DIP should explain, preferably with examples, what problems would be caused by allowing the compiler to accept these calls--i.e., what "algorithms and user code" would need to be changed, and how burdensome those changes would be.

There are two issues at play here.

1) Introspection of the function declaration
2) Function calling

Function calling is straight forward. Named parameters and hence named arguments are not taken into account with symbol/overload resolution. There is a requirements section dedicated to this.

Introspection is a lot more interesting. A good example of this is std.traits : isInstanceOf. And that is easily solved by using a string mixin to map the named parameters from T to S.

That is noted since that would be a good example of how simple it could potentially be. But I'm not sure I can do the opposite for complex (since then we are talking e.g. ORM's),
https://dlang.org/phobos/std_traits.html#.isInstanceOf

>> Template declarations that have named parameters expose the named parameter as a member of the template.
> 
> This feature is not essential to the DIP, so its inclusion needs to be justified separately.

It is essential otherwise the DIP will need a full redesign.

>> Manifest constant templates and templated aliases are treated as if they were
>> eponymous templates and do expose named parameters as members.
> 
> Non-eponymous members of eponymous templates are currently inaccessible from outside the template body, so this will require changes to D's name lookup rules. Example:
> 
>     template size(T_)
>     {
>         alias T = T_;
>         enum size = T_.sizeof;
>     }
> 
>     // Error: no property T for type ulong
>     pragma(msg, size!int.T);

Dangit, noted.

>> * named arguments at the end of an argument list may appear in any order
>>
>> * named arguments may be interleaved with positional arguments out of
>> sequence, but if there are more than one they must match the order in which
>> the named parameters are declared
> 
> The DIP should explain why these particular reordering rules were chosen instead of any of the alternatives from previous proposals, or from the examples given in the "Prior Work" section.

That means I need to add a new section and bring back some content from the pre-editing no/partial/full reordering sections about their pros/cons.


Actionable list available here: https://gist.github.com/rikkimax/91d0291da30d6ed161c6d3fa89868883

Thanks!
September 11, 2019
On 11/09/2019 6:11 AM, 12345swordy wrote:
> On Tuesday, 10 September 2019 at 16:09:39 UTC, rikki cattermole wrote:
>> On 11/09/2019 3:45 AM, 12345swordy wrote:
>>> On Tuesday, 10 September 2019 at 14:19:02 UTC, rikki cattermole wrote:
>>>> On 11/09/2019 2:01 AM, 12345swordy wrote:
>>>>> Issues with the "@named" attribute:
>>>>> 1.This is opt-in rather then opt-out, which may causes users to beg the library maintainers to update their libraries to support name attribute.
>>>>
>>>> This is intended.
>>>> If you want to override the intention of the API author then we need to find another solution. One which I am unsure about how good it would be.
>>>
>>> Make it an opt-in compiler flag.
>>>
>>> Alex
>>
>> I do not believe a flag like this is a good solution.
>> Over the years there has been a lot of talk about how bad -property is and it is similar in function to what you are proposing.
> 
> The issue with property atm, is that is half-baked. People keep thinking that optional parentheses will replace it. It didn't as it doesn't support operators such as += and ++. This flag deals with the interaction with other libraries and public interfaces.

I remain against it. However I will think about what you are saying with the flag. Even if I think the syntax ``! Identifier :`` to match against both named and unnamed parameters would cover you instead better.

>> However I do have a syntax in mind if there is an overwhelming call to support overriding the API makers intention for an API.
>>
>> The problem with it is, I have no idea how it would be implemented. But Walter seems to, so if the desire is there by enough people I guess I would have to add it.
> 
> Would it be better to have the named arguments opt-out via @unnamed instead of opt-in via @named? If certain people are that worry about breaking the public API, they should avoid mark their function as "@unnamed", instead having to mark every function @named just to use named arguments.

Positional should remain the default (otherwise you risk changing how existing code gets thought about).

Anyway, I doubt anyone would use such an attribute.

I see named arguments used sparingly when the abstractions need it. I would question any code that used named parameters so heavily as to have them on every single parameter in quite large files.

> Besides we are dealing with c and c++ external bindings, and there are tools that generate those bindings. If I were to modify the tools to generate the @named arguments automatically, I still may end up in a plausable situation where the c/c++ library managers may changed the arguments names, which then causes breakage in the public api. Which it renders the case for @named moot, as you are not necessarily the developer for those c/c++ libraries.

This DIP does not limit named parameters to extern(D) code. Since there may be valid reasons to need to extern(C) it within a code base.

But I do not intend for this feature to be used with other languages. Since their API authors you are binding to could not intend for their API to work with it.

> This dip fail to provide a solid case where breakage from the change of the public api function arguments is a huge deal that needs to be avoided.
> 
> - Alex

Actually right now I think DIP 1020 has the potential to have breakage from the change of API named parameter names. It alone provides no tools to deal with it.

Apart from designing a little defensively, I am not taking a stance on this problem in the DIP. But I think I should consider what you are saying. I think there is something I should take note of here but I'm not sure what it is just yet.
September 10, 2019
On Tuesday, 10 September 2019 at 09:06:23 UTC, Mike Parker wrote:
> This is the feedback thread for the second round of Community Review for DIP 1020, "Named Parameters":
>
> https://github.com/dlang/DIPs/blob/c723d8f4e3ac2d5bcaf8cdff87e1507f09669ca6/DIPs/DIP1020.md
>
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on September 24, or when I make a post declaring it complete.
>
> At the end of Round 2, if further review is deemed necessary, the DIP will be scheduled for another round of Community Review. Otherwise, it will be queued for the Final Review and Formal Assessment.
>
> Anyone intending to post feedback in this thread is expected to be familiar with the reviewer guidelines:
>
> https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md
>
> *Please stay on topic!*
>
> Thanks in advance to all who participate.

Sorry in advance for the bikeshedding, but using the @ symbol just looks ugly to me. How about using 'public' instead of '@named'?

    void foo(@named bool log, @named bool goFast);
Vs.
    void foo(public bool log, public bool goFast);
September 10, 2019
On Tuesday, 10 September 2019 at 18:47:54 UTC, Jonathan Marler wrote:
> On Tuesday, 10 September 2019 at 09:06:23 UTC, Mike Parker wrote:
>> [...]
>
> Sorry in advance for the bikeshedding, but using the @ symbol just looks ugly to me. How about using 'public' instead of '@named'?
>
>     void foo(@named bool log, @named bool goFast);
> Vs.
>     void foo(public bool log, public bool goFast);

I like that, both syntactically and also because the former would break anyone with UDAs called @named
September 10, 2019
On Tuesday, 10 September 2019 at 18:47:54 UTC, Jonathan Marler wrote:
> On Tuesday, 10 September 2019 at 09:06:23 UTC, Mike Parker wrote:
>> [...]
>
> Sorry in advance for the bikeshedding, but using the @ symbol just looks ugly to me. How about using 'public' instead of '@named'?
>
>     void foo(@named bool log, @named bool goFast);
> Vs.
>     void foo(public bool log, public bool goFast);

In fact, @public cannot compile as is so that might be a better solution still (bare "public" might require modifying the parser more but it's not my expertise)
September 10, 2019
On Tuesday, 10 September 2019 at 19:34:45 UTC, Max Haughton wrote:
> On Tuesday, 10 September 2019 at 18:47:54 UTC, Jonathan Marler wrote:
>> On Tuesday, 10 September 2019 at 09:06:23 UTC, Mike Parker wrote:
>>> [...]
>>
>> Sorry in advance for the bikeshedding, but using the @ symbol just looks ugly to me. How about using 'public' instead of '@named'?
>>
>>     void foo(@named bool log, @named bool goFast);
>> Vs.
>>     void foo(public bool log, public bool goFast);
>
> In fact, @public cannot compile as is so that might be a better solution still (bare "public" might require modifying the parser more but it's not my expertise)

Yes using `public` would require a grammar change.  However, the feature requires other grammar changes as well.  Also, using either `public` or `@named` without the feature will result in compilation errors so they're both on the same level in that regard.
September 10, 2019
On Tuesday, 10 September 2019 at 18:41:50 UTC, rikki cattermole wrote:
> [snip]
>
> Actually right now I think DIP 1020 has the potential to have breakage from the change of API named parameter names. It alone provides no tools to deal with it.
>
> Apart from designing a little defensively, I am not taking a stance on this problem in the DIP. But I think I should consider what you are saying. I think there is something I should take note of here but I'm not sure what it is just yet.


I would think that this issue would affect any named parameter DIP. Library users would need to rely on the parameter names not changing if they make use of them. The most common tool would be grep. There's no reason that library authors couldn't use a deprecation period where they add another parameter to the function with the new name and have the old named one forward to it.

Also, the DIP has a section about the review and includes a mention to Walter's suggestion, but I don't really see an explanation of why this DIP is better than his suggestion. In general, I like the idea of named parameters, but I find his version more parsimonious until convinced otherwise.
September 10, 2019
On Tuesday, 10 September 2019 at 20:08:38 UTC, jmh530 wrote:
> [snip]

I also don't understand this

void foo(int a) {
}

void foo(int a, @named int b) {
}

[snip]

foo(1); // error: matches both declarations
foo(1, b: 2); // error: matches both declarations


September 10, 2019
On 9/10/2019 2:06 AM, Mike Parker wrote:
> This is the feedback thread for the second round of Community Review for DIP 1020, "Named Parameters":
> 
> https://github.com/dlang/DIPs/blob/c723d8f4e3ac2d5bcaf8cdff87e1507f09669ca6/DIPs/DIP1020.md 

My review is the same as in the first round:

https://digitalmars.com/d/archives/digitalmars/D/DIP_1020--Named_Parameters--Community_Review_Round_1_325299.html#N325627

To which Rikki responded with:

"That solves function arguments, what is your proposal for templates?"

https://digitalmars.com/d/archives/digitalmars/D/DIP_1020--Named_Parameters--Community_Review_Round_1_325299.html#N325631
September 10, 2019
On Tuesday, 10 September 2019 at 20:33:01 UTC, jmh530 wrote:
> On Tuesday, 10 September 2019 at 20:08:38 UTC, jmh530 wrote:
>> [snip]
>
> I also don't understand this
>
> void foo(int a) {
> }
>
> void foo(int a, @named int b) {
> }
>
> [snip]
>
> foo(1); // error: matches both declarations
> foo(1, b: 2); // error: matches both declarations

The idea is that named arguments and named parameters are ignored *completely* when resolving overloads--only the positional arguments are used. So it's as if you'd written:

void foo(int a) {
}

void foo(int a) {
}

foo(1);
foo(1);
1 2 3 4 5 6 7 8 9 10