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

[Revived] In-place struct initialization #71

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
@wilzbach
Copy link
Member

commented Jun 8, 2017

This is a revival of #22, because I thought the idea is cool and I often could profit from the changes of this DIP.

I am not an expert on the D grammar, so @ everyone, please feel to criticize the proposed additions.

CC @cym13 @Enamex

As this is an adopted PR, help to polish it is more than welcome.


Example:

auto s = S({a:42, b:-5});

This comment has been minimized.

Copy link
@jacob-carlborg

jacob-carlborg Jun 11, 2017

Contributor

In my opinion this looks like passing a single value to the constructor of S. Perhaps I'm just used to Ruby, where the above is an associative array.

This comment has been minimized.

Copy link
@wilzbach

wilzbach Jun 13, 2017

Author Member

Hmm I just realized that this is ambiguous with anonoymous FunctionLiterals:

    AST.Expression parsePrimaryExp()
    {
...
        case TOKlcurly:
        case TOKfunction:
        case TOKdelegate:
        case_delegate:
            {
                AST.Dsymbol s = parseFunctionLiteral();
                e = new AST.FuncExp(loc, s);
                break;
}

[Source](https://github.com/dlang/dmd/blob/master/src/ddmd/parse.d#L7559

This comment has been minimized.

Copy link
@schuetzm

schuetzm Jul 16, 2017

No, the only ambiguous case is {}, and it can be arbitrarily resolved to a function literal to keep backwards compatibility. Non-empty function literals (with braces) always contain statements, which either end in a semicolon, or contain a keyword (e.g. struct).


auto s = S([a:42, b:-5]);

This syntax may potentially be ambiguous with associative arrays.

This comment has been minimized.

Copy link
@jacob-carlborg

jacob-carlborg Jun 11, 2017

Contributor

I'm pretty sure this is true. Not sure if it's worth having as a proposal at all.


Example:

s = S(c:10, b:20);

This comment has been minimized.

Copy link
@jacob-carlborg

jacob-carlborg Jun 11, 2017

Contributor

In my opinion this is the best syntax.

This comment has been minimized.

Copy link
@bausshf

bausshf Mar 9, 2019

Contributor

This would work if D allowed named arguments.

@jacob-carlborg

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2017

I think the focus should be on the inconsistency in the language, that is, that there's this very specific syntax { foo: 3 } that only works in one particular case. Rather than focusing on named arguments.


To understand the proposed changes in a short recap the interesting parts of
D's grammar will be highlighted.
Currently [static struct initialization](http://dlang.org/spec/grammar.html#StuctInitializer)

This comment has been minimized.

Copy link
@ntrel

ntrel Jun 11, 2017

Contributor

typo: #StructInitializer

assert(args!(fun, b=>3) == 16);
```
While this is very nice trick, it has a few downsides:

This comment has been minimized.

Copy link
@ntrel

ntrel Jun 11, 2017

Contributor

I think a major reason is that it relies on function parameter names not changing, i.e. they become part of the library API. Named arguments need to be distinguished from normal parameters, which can be renamed - this DIP solves this problem.

.open;
```
Even though this is a nice Java style, allocating a class and function calls don't come for free.

This comment has been minimized.

Copy link
@ntrel

ntrel Jun 11, 2017

Contributor

Nitpick: sounds like a struct would be better rather than class for stack allocation. I don't know but maybe the optimizer can reduce the function calls to the equivalent of struct initialization. But one problem with this pattern is that it's a bit tedious to write those methods by hand.

aes!("x", "y")(dat1.value1, dat1.value2)
```

#### Vulkan

This comment has been minimized.

Copy link
@ntrel

ntrel Jun 11, 2017

Contributor

This heading and the next two should use ### for level-3, they're not part of Plotting.

### Links

- [D grammar](https://dlang.org/spec/grammar.html)
- [Language specs on structs](https://dlang.org/spec/struct.html)

This comment has been minimized.

Copy link
@ntrel

ntrel Jun 11, 2017

Contributor

Maybe change this (or add a link) to static struct initialization spec

@zachthemystic

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2017

My experience writing #66 gave me some insight into how a persuasive DIP runs in the general case. I'm copying this comment I made for #61 , as it applies here too. My preferred order for DIP presentation is:

Section: Rationale (i.e. State the Problem)
What you want to do/be able to do, and why. What you currently must do to achieve the effect in question. Code example. Why a better solution is desirable. Who is the "customer" for the better way?

Section: Description
Now describe your proposed feature.

Section: Drawbacks and Alternatives (drawbacks, i.e. breakage, language complexity, corner cases)
Be as honest as possible. Try to argue the opposition's case for them. Imagine you're a language designer trying to find reasons not to include yet another feature. List all known viable alternatives.

@zachthemystic

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2017

I think the grammar descriptions and AST sections are particularly unpersuasive. The grammar should be moved to the bottom of the DIP, and only kept because it's necessary for a complete proposal. But I think the AST sections can be completely removed. I really can't imagine anyone being persuaded by them. If there are any parsing problems, they should be addressed somewhere else.

@wilzbach

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2017

@jacob-carlborg @ntrel @zachthemystic thanks a lot for your helpful feedback - I am just trying my best at reviving the best, so please have a bit of patience.

@jacob-carlborg

I think the focus should be on the inconsistency in the language, that is, that there's this very specific syntax { foo: 3 } that only works in one particular case. Rather than focusing on named arguments.

Fair point - I tried to reword it, but it still needs more. Feel free to point out sections that you thought weren't helpful.

@zachthemystic: thanks a lot for the idea - I restructured the page and put the rationale after the description.

Try to argue the opposition's case for them. Imagine you're a language designer trying to find reasons not to include yet another feature. List all known viable alternatives.

Except for the workarounds with named arguments, I couldn't a reason not to allow in-place struct initialization anywhere. Ideas?

I think the grammar descriptions and AST sections are particularly unpersuasive.

Thanks - what in your opinion could I add to help in convincing a reader?

But I think the AST sections can be completely removed. I really can't imagine anyone being persuaded by them. If there are any parsing problems, they should be addressed somewhere else.

Hmm I found it very helpful to understand the consequences of the grammar changes - what do other people think on this?

@zachthemystic

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2017

My first approach is as a general reader. I think the technical decisions can be very difficult, as there are many pros and cons, which must be weighed very accurately. So I'm starting by ignoring the technical details, trying to assess the quality of the writing for its persuasiveness. The following are my opinions, and I don't consider them absolutely right or wrong.

The Abstract should be punchier, starting with the 2nd sentence. For example: "This DIP proposes to expand support for static struct initialization to any place where calling a struct constructor would be possible." Imagine you only have one sentence to grab the reader's attention. If you absolutely need more sentences, then okay, but the first one needs to say concisely and in active terms what the DIP does.

The Links section is okay, not because it grabs, but simply because it's necessary.

So then I get to Description, and I'm already kind of losing interest. I need to be convinced that there's a problem. Imagine the reader has a thousand other things to do than read the DIP (which is probably true). What can you say to convince them there is an important problem here that is worth their attention? That is why I would rather read the rationale at this point. Only when I'm convinced there's a problem do I need to see the solution.

The existing Rationale which follows does not actually do this, in the sense that it promotes benefits without making the problem felt.

I realize that none of what I'm saying actually addresses technical concerns. But I have enough confidence that whoever reads this, including the language authors, are human beings, who are always affected by many things other than pure logic, which is why I'm saying it.

I'll continue to review if and when the above comments are addressed.

@andre2007

This comment has been minimized.

Copy link

commented Jun 14, 2017

the work on this dip is highly appreciated. For my AWS SDK this DIP would
make the coding much more readable and also smaller for several use cases.

I generate structures out of the AWS API information. Several UDA information has to be stored. Struct initializer for UDA structures will look great:

struct CreateTableInput
{
	@FieldInfo({memberName: "TableName"})
	TableName tableName;

	@FieldInfo({memberName: "AttributeDefinitions", minLength: 1})
	AttributeDefinitions attributeDefinitions;
}

Second scenario is the actual usage of these structs. Using struct initializer in method signature feels natural:

invoker.execute([
	new CreateBucketCommand(client, {
		bucket: "MyBucket1",
		createBucketConfiguration: {
			locationConstraint: BucketLocationConstraint.EU_CENTRAL_1
		}
	}),
	new CreateBucketCommand(client, {
		bucket: "MyBucket2",
		createBucketConfiguration: {
			locationConstraint: BucketLocationConstraint.EU_CENTRAL_1
		}
	})
]);

@wilzbach wilzbach referenced this pull request Jun 15, 2017

Open

API Overhaul #87

@mdparker

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2017

@wilzbach I'm going to agree with @jacob-carlborg about focus. As it reads now, it sounds more like a DIP for named arguments than for struct initialization. IMO, consistency is the key selling point here, as is the principle of least surprise; if it works in declarations, it's reasonable to expect that it should work elsewhere and surprising that it doesn't. And if memory serves, Walter is against named arguments (there have been multiple discussions in the forums), so repeating the phrase so frequently may actually be counter-productive!

@rtbo

This comment has been minimized.

Copy link

commented Jun 25, 2017

It's a nice DIP IMO. I sometimes have the case of defavoring struct constructor syntax in favor of field by field assignment for the sake of readability.

Consistency with the static assignment syntax would be to accept this very syntax everywhere a struct can be initialized:

struct S {
    uint a;
    long b;
    int  c;
}
struct SS {
    uint d;
    S s;
}
void foo (int e, S s);
void bar (SS ss, int f);

S s = { a: 42, b: -5 };  // c initialized as int.init
SS ss = { s: { a: 42, b: -5 } };

foo(12, { a: 42, b: -5 });
bar({ s: { a: 42, b: -5 } }, 54);

// without forgetting default arguments
// following a and b known at compile time, previous ones can be evaluated at run time
void baz (int g, S s = { a: 42, b: -5 });

How about the case of a template argument? It should be possible to optionally specify the type:

void tfoo(T)(int e, T s);
tfoo(12, S { a: 42, b: -5 });

The S { ... } syntax is kind of loosing consistency with the regular constructor syntax, but impossible to infer the types in template instantiation context otherwise, and a regular S ( ... ) syntax would open the door to the persona non grata named arguments.

In that case, 2 options:

  • type name mandatory for every struct named field initialization
  • type name optional but allowed everywhere (mandatory only for template arguments)

I prefer the latter:

  • not breaking currently allowed syntax
  • S is not verbose, but TemplateStruct!(Type1, "value 2") is.
@narfanar

This comment has been minimized.

Copy link

commented Jun 28, 2017

Here's an idea to get around having either named argument-syntax for this purpose or something that conflicts with q{} strings:

There was an old proposal that I faintly remember about having {arg0, arg1, ...} syntax for tuples. The idea is: Something like this for tuples with named arguments.

So we could then have:

auto my_anon_struct = {foo: 42, bar: 89}; // type == Tuple!(int, "foo", int, "bar")
                                          // or something like it
auto my_var = {foo: 42, bar: 89}.as!MyStruct; // type == MyStruct

pragma(inline, true)
auto as(T, U)(auto ref U u) {
    T t = mixin(getTupleLiteral!(U, "u"));
    // This gives us something like:
    // T t = {foo: u.foo, bar: u.bar};
    // Pretty straightforward so hopefully the compiler can
    // reliably inline
    return t;
}
@narfanar

This comment has been minimized.

Copy link

commented Jul 12, 2017

Might be of interest in relation to this 'anonymous records as field-named tuples' suggestion:

https://www.microsoft.com/en-us/research/wp-content/uploads/1999/01/recpro.pdf
(this paper talks in the context of Haskell; ignoring the extensibility aspect, I think it's relevant and interesting).

I don't mean to sideline the suggestions laid out in this proposal. This's merely the only place I know of right now that has an ongoing discussion about it and I'm not sure about starting an NG thread for this while a proposal is alive.

@wilzbach

This comment has been minimized.

Copy link
Member Author

commented Jul 14, 2017

Found this thread on the NG: http://forum.dlang.org/post/ok9nt7$2e5d$1@digitalmars.com

@ others thanks a lot for your feedback. Highly appreciated, but unfortunately I won't have much time for this DIP this month, so if you want to push it feel free to open PRs against my branch.

return s;
}
playWithS(createS(b=3));
```

This comment has been minimized.

Copy link
@sysint64

sysint64 Oct 26, 2017

no syntax highlighting

@wilzbach

This comment has been minimized.

Copy link
Member Author

commented Mar 15, 2018

In the State of D survey, in-place struct initialization was the fourth-most missed language feature with 28% of all respondents missing it:

image

https://rawgit.com/wilzbach/state-of-d/master/report.html

@jacob-carlborg

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2018

In the State of D survey, in-place struct initialization was the fourth-most missed language feature with 28% of all respondents missing it

So what are we waiting for 😃.

@FeepingCreature

This comment has been minimized.

Copy link

commented Jun 7, 2018

Yeah, what are you waiting for?

@cym13

This comment has been minimized.

Copy link

commented Jun 7, 2018

I think the main blocker is the lack of consensus on a syntax. Not sure how to go about it, people can just shout what they think is the best idea but it won't amount to much, and bringing that issue to the forums would be... let's call that a risky bet. Some kind of structured poll maybe?

@wilzbach wilzbach force-pushed the wilzbach:struct-initialization branch 2 times, most recently from 5d2a21a to 4c254d7 Jul 2, 2018

@wilzbach

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2018

So what are we waiting for smiley.
Yeah, what are you waiting for?

Well I was quite busy and somehow no one else pushed this further :/
Also I wanted to implement this in DMD to show that it's absolutely possible to do, know that my proposed grammar changes are actually correct and increase the rate of acceptance of this DIP.
As I got this working in DMD (it's still pretty ugly, so the PR will follow later) I'm feeling more confident to finally start submitting this PR to DIP queue.
I made an entire overhaul of this DIP, so any feedback on this reworked version is very welcome!

I think the main blocker is the lack of consensus on a syntax.

AFAICT

  • Option 3 (Foo{a: 1} would only be possible in a more restricted way and isn't well-liked anyhow
  • Option 2 - that's potentially conflicting with named arguments (see e.g. #126). I think it could be done without conflicts, but I'm not sure which named arguments DIP will be accepted.

So chances are that we will end up with Option 1, but I expected this issue to be discussed in-depth in the first review stage of this DIP.

@jacob-carlborg

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2018

Option 1 could just as well be in conflict with tuples with names.

AttributeDefinitions attributeDefinitions;
}
```

This comment has been minimized.

Copy link
@jacob-carlborg

jacob-carlborg Jul 2, 2018

Contributor

I think another great example would be to create a struct instance to be converted to JSON in an implementation of a HTTP API endpoint.

@jacob-carlborg

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2018

I like that revived proposal manly focuses on the current inconsistencies/limitations rather than named arguments. 👍

@rikkimax

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2018

@wilzbach Option 2 will definitely conflict with named arguments, given my DIP and others like 66.

But there is another problem with 2.

void foo(Bar a, Bar b);
foo(a:1, b:2, a:1:, b:3);

The arguments should be grouped anyway, otherwise it will introduce restrictions which are quite artificial. But fundamentally what I'm seeing is named parameters as API which is what my DIP is trying to achieve. With an additional edge case.

However the problem I am seeing with this DIP currently is ambiguity with templates aka what happens when type deduction fails; I do expect that to be asked about on D.learn quite a bit, especially early on.

I think more experimentation on the declaration side might be a good way to go. After all, the goal is to change what .init is for a type from the outside of it, in select locations.

@jacob-carlborg

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

BTW, C++11 supports this, using the following syntax:

struct Foo
{
    int a;
    int b;
};

auto a = Foo{ .b = 4 };

@wilzbach wilzbach force-pushed the wilzbach:struct-initialization branch from 4c254d7 to bf94283 Jul 6, 2018

@wilzbach wilzbach force-pushed the wilzbach:struct-initialization branch from bf94283 to 26c7274 Jul 6, 2018

@wilzbach

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2018

As I got this working in DMD (it's still pretty ugly, so the PR will follow later)

-> dlang/dmd#8460
It's still ugly and just a PoC, but I hope it allows to understand this proposal better

However the problem I am seeing with this DIP currently is ambiguity with templates aka what happens when type deduction fails; I do expect that to be asked about on D.learn quite a bit, especially early on.

Does this also apply to Option 1? (the default option of this DIP and the option used for the PoC DMD PR).

@mdparker is there a slot in the DIP queue available at the moment and what do you think still needs to be done for an initial submission?

@wilzbach wilzbach force-pushed the wilzbach:struct-initialization branch from 26c7274 to baae3e4 Jul 6, 2018

@rikkimax

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2018

@wilzbach looks like my analysis was incorrect. This statement was less than adequately explained in examples I think "In-place struct initialization behaves analogous to default constructor of structs and thus is only allowed when there's no user-defined constructor.". So type deduction shouldn't be an issue after all. But a concern would be, are we calling a constructor or doing initialization?

Of course my immediate assumption that 1 and 2 would have some sort of function calling behavior applied to it, is concerning.

@wilzbach wilzbach force-pushed the wilzbach:struct-initialization branch from baae3e4 to 927864b Jul 6, 2018

@wilzbach

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2018

But a concern would be, are we calling a constructor or doing initialization?

Initialization. FWIW with dlang/dmd#8460 foo and foo2 are equivalent and do generate the same assembly:

struct S
{
    int a = 2, b = 4, c = 6;
}
void foo()
{
    bar(S({c: 10}));
}
void foo2()
{
    S s = {c: 10};
    bar(s);
}
void bar(S);
0000000000000000 <_D3fooQeFZv>:
   0:	55                   	push   %rbp
   1:	48 8b ec             	mov    %rsp,%rbp
   4:	48 83 ec 10          	sub    $0x10,%rsp
   8:	c7 45 f0 02 00 00 00 	movl   $0x2,-0x10(%rbp)
   f:	c7 45 f4 04 00 00 00 	movl   $0x4,-0xc(%rbp)
  16:	c7 45 f8 0a 00 00 00 	movl   $0xa,-0x8(%rbp)
  1d:	48 8b 55 f8          	mov    -0x8(%rbp),%rdx
  21:	48 8b 7d f0          	mov    -0x10(%rbp),%rdi
  25:	48 89 d6             	mov    %rdx,%rsi
  28:	e8 00 00 00 00       	callq  2d <_D3fooQeFZv+0x2d>
  2d:	c9                   	leaveq 
  2e:	c3                   	retq   
@jacob-carlborg

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2018

Regarding the third option is ambiguous with token strings. We already have the problem with attributes and UDAs. That is, I can declare a struct with the name property but I cannot use it as a UDA because that conflicts with the built-in attribute. I don't have a problem using the third option.

@wilzbach wilzbach force-pushed the wilzbach:struct-initialization branch from 927864b to b1283b4 Jul 23, 2018

@cym13

This comment has been minimized.

Copy link

commented Jul 23, 2018

One question: since we're talking about initialization and not construction, should the following be legal (assuming syntax option 1)?

struct S {
    int a;
    this(int i) { this.a = i; }
}

S mystruct = S({a: 12})(13); // Calls the int constructor of the struct of type S initialized with a=12
assert(mystruct.a == 13);

If not, how should that be justified? If so, should there be any form of restriction or something?

@jacob-carlborg

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2018

One question: since we're talking about initialization and not construction, should the following be legal (assuming syntax option 1)?

@cym13 No. Since if you define a constructor the initializer syntax cannot be used:

struct Foo
{
    int a;
    this(int a) {}
}

void main()
{
    Foo f = { a: 3 };
}
Error: struct `Foo` has constructors, cannot use `{ initializers }`, use `Foo( initializers )` instead
@wilzbach

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2018

For reference the most recent discussion on the forum about this DIP was here: https://forum.dlang.org/post/lpixarbirhorkltaqlew@forum.dlang.org

@dukc

This comment has been minimized.

Copy link

commented Sep 12, 2018

#126 does not conflict with syntax option 2, as I see it. The reason is that that #126 states that named parameters do not affect overloading resolution. This means that a constructor call where all parameters are named has same overloading as a constructor without parameters -and defining one for structs is currently forbidden. No conflict.

If this is extended to classes, there will be conflict with default constructors, but I don't see it as a problem because it won't break anything. Just disallow in-place initialization for classes that have an explicit default constructor.

@marler8997 marler8997 referenced this pull request Dec 11, 2018

Open

Issue Bucket #43

Show resolved Hide resolved DIPs/DIP1xxx-sw.md Outdated

@wilzbach wilzbach force-pushed the wilzbach:struct-initialization branch from b1283b4 to 3068c4b Mar 4, 2019

@andre2007

This comment has been minimized.

Copy link

commented Mar 31, 2019

In all 3 options, the name of the struct is mentioned. I wonder wheter in addition it would be possible leave out the struct name in case the compiler could detect there is only 1 distinct structure which fits. For example a function "foo" only accepts structure "A" it would be possible to just write
foo({bar: ...}};
Or to write it with the structure name A.
foo(A{bar: ...}};

@andre2007

This comment has been minimized.

Copy link

commented Apr 1, 2019

I just saw, it is already described in "bonus 1" :)

@Bolpat

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

The syntax auto a = Foo{ .b = 4 }; can be conflated with the assignment to some global b while initializing a. In D, the notation .b usually means the global b; note that C++ uses ::b for that.

I'd go with plain { b: 4 }, and if the compiler cannot infer the required type, use a the type
Foo{ b: 4 }
or a cast
cast(Foo){ b: 4 }
It looks ugly, but how often do you need it?

It's the option most consistent with the current state:
Foo foo = { b: 4 };
to
auto foo = Foo{ b: 4 };

I really dislike the parentheses since it is not a constructor call.

If the struct happens to be named q, you can

  • use a space: q { b: 4 }
  • use an alias: alias r = q; then r{ b: 4 }.
  • use a cast: cast(q){ b: 4 }

to avoid building a string literal.

@andre2007

This comment has been minimized.

Copy link

commented Apr 7, 2019

There is a proposal from Walter which goes also into this direction
https://forum.dlang.org/post/q8bpkl$fsq$1@digitalmars.com

Quote:
One nice thing about this is the { } struct initialization syntax can be deprecated, as S(a:1, b:2) can replace it, and would be interchangeable with constructor syntax, making for a nice unification. (Like was done for array initialization.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.