Should copy initializers be generated by default, even when other explicit initializers are present?

[lydia-duncan]

There are a lot of situations where a copy initializer is required, but today the presence of an explicit initializer prevents it from being generated.

Example 1:

record Foo {
  proc init() {
    super.init();
  }
}
proc quux(x: Foo) return x; // Returning an argument requires a copy initializer
var foo = new Foo();
var bar = quux(foo);
writeln(foo);
writeln(bar);

Example 2:

record Foo {
  var x : int;
  var y : real;

  proc init() {
    x = 5;
    y = 10.0;
  }
}
var A : [1..4] Foo; // Creating an array of records requires a copy initializer, currently
writeln(A);

Should we always generate a copy initializer unless we recognize that an explicit one has been defined?

Should this be connected to whether = or a non-copy initializer was provided?

[lydia-duncan]

@mppf - how does this look?

[noakesmichael]

I have been assuming that this would prove to be desirable at some point and I support this direction.

[mppf]

It seems inconsistent to me right now that we generate the assignment operator by default, even if an initializer is provided, but we don't generate a copy initializer by default.

I think it'd be reasonable to generate both assignment and the copy initializer by default; if the user doesn't want the default one they have to write a copy initializer. Since the compiler/modules generally expects that = and copy initialization are possible, these should probably generally be generated by default.

It's certainly the case for Owned that we don't (currently) want to generate or call auto-copies, but that's almost certainly the exception to the rule - that's the only record I know of in the situation of not wanting any copy-initializer defined, and in the future it might be in a different position there (with more compile-time checking of Owned / borrow checking etc).

One day, I hope that simply providing a copy-initializer that calls compilerError would be sufficient for a record that doesn't want to be copyable. That's not easily achievable today with the autoCopyMap though - and I'd rather have a good default than have good support for the weird case of no copy initializer for a record. (In the meantime, we have a flag/pragma that Owned uses). (Edit -- actually supporting that pattern might not be that hard, it'd just be more weird cases in resolution to do it the obvious way).

[bradcray]

I'd expect a default copy initializer to be created for records, but to be shadowed by a user-specified initializer that "fit" the expected signature as described in my comments on issue #8064.

[lydia-duncan]

Closing. The current decision is "yes, copy initializers should be generated by default"

[bradcray]

Based on experience converting module code over, I'm wondering if we made the wrong decision here. As initializers currently stand, I'm only somewhat convinced we did. If we were to split the phase 1 and phase 2 portions of initializers apart, as is being explored in issue #8018 then I feel more confident that reversing this decision might be correct.

In more detail: In converting a complex initializer case over (e.g., one that had deep/shallow copy and ownership issues), I converted the existing constructors and added a few new overloads that were required in the new world, but did not add a copy initializer. The compiler provided one, but because of the parenthetical issues above, it was inherently broken. Because of the decision made on this issue, however, the result was that the program blew up which was difficult to diagnose.

If instead, the presence of the custom initializers had disabled the automatic copy initializer, it would've forced me to think about what I should do in that case and made it less likely that I would've stepped into the hole. If there were a mechanism for opting into having the compiler provide such an initializer for use (i.e., "fulfill my need for a copy initializer by having the compiler provide it") then the effort would've been low for other cases.

As I write this, something I'm wondering about is what the cues might be for "the compiler's default initializer is unlikely to cut it." In our current "initializers contain phase 1 and 2 code" world, a challenge with reversing this decision is that initializers may contain all sorts of code that is unrelated to questions of ownership, deep/shallow copies, etc. E.g., maybe they have a check and halt which required a user initializer, but didn't require a custom copy initializer. So something I'm wondering about is whether the cue for bringing the user initializer into the mix might be something different like "presence of an assignment overload" or "presence of a custom phase 1 initializer, but not a custom phase 2 initializer" or ... I'm still thinking about this question.

[lydia-duncan]

What if we only prevented the generation of a default copy initializer in the case where an explicit assignment function was provided for the type?

[bradcray]

Yeah, that's one of the options I was considering (see "So something I'm wondering about..." above). OTOH, I could imagine the situation reversed: maybe the compiler shouldn't create a default assignment for the case when there was a copy initializer (or shouldn't create either in the event of ...something else...?)

[mppf]

I think it'd be reasonable to require record authors to accept the compiler's copy initializer and assignment function or implement them both. I.e. it would be an error to provide assignment and no copy initializer or to provide copy initializer and not assignment.

A somewhat reasonable alternative would be for the default compiler-generated one to call the one the user provided, but that can create some weird cases... (e.g. we could implement channel assignment out of init copy or implement channel init copy out of assignment... but I worry this won't generalize well to arbitrary records with fields with their own copy/assignment).

[lydia-duncan]

It's best to start from less permissive and move to more permissive, anyways, so requiring both to provided if one is a good starting point if we don't feel we have enough information yet.

[bradcray]

I'm reopening this based on recent experiences and discussion.

[mppf]

I'm running into problems related to this issue as part of working on https://github.com/Cray/chapel-private/issues/667 / #13600.

I need a way for a type like list(owned C) to indicate to the compiler that init= and = should both result in an error when called.

[mppf]

I would expect that if there is any user-provided init= that there would not be a compiler-generated init= for that type (similarly to how we handle init).

Additionally we need a way to have an init= that the user writes to indicate that the type is not copy-initializable. This could be:

• an init= body containing compilerError
• new syntax, like proc init=(other) = none
• an init= that can never be resolved and relying on the above so that a compiler-generated one is not preferred (e.g. proc init=() { } or proc init=(other) where false { })

There is a similar question for the assignment operator. For the assignment operator, I'm currently thinking that the compiler would generate one only if there isn't a user-provided init= for the type, and later on in compilation, make a compilation error if there is a user-provided = for a type but it uses the compiler-generated init=.

Users will need a way to indicate that the type does not support assignment. That could be:

• an invalid init= and no = operator (relying on the rule above - the compiler would not use a default = in this case because init= was provided)
• an = containing a compilerError
• new syntax, like proc =(ref lhs:R, rhs:R) = none
• an = overload that can never be resolved (but.. would the compiler-generated one be preferred?) e.g. proc =(ref lhs:R, rhs:R) where false

Note that the new syntax options like proc init=(other) = none and proc =(ref lhs:R, rhs:R) = none could have syntax that overlaps with the way to declare a pure virtual method - see #8566

[mppf]

If we wanted new syntax that conceptually solved both #8136 and #8566, we could add:

• e.g. proc init=(_) = none to indicate there is no init=
• e.g. proc method() = override to indicate a pure virtual method
• e.g. proc init(_) = default to request the compiler-generated default initializer.

(I think one of the main complexities with requesting the default init/init= is that we wouldn't want to put an argument list since repeating the argument list would be much of the work in manually writing the default init).

(Also the = after init= might be unfortunate - consider proc init= = none vs proc init = none... This is arguably better if we put some kind of syntactical placeholder in for the argument list).

Of course that's not the only approach to solving this issue.... but such a syntax seems fairly unambiguous/clear to me while allowing flexibility for the different things we want to do.

[mppf]

I'm proposing:

(A) Make it an error to specify one of init= and = but to rely on the compiler-generated default for the other.

(B) Adjust the compiler to avoid generating init= for a type if there is a user-provided init= (similarly to how we handle init).

(C) Provide a way for users to indicate that a type has no functional init= (that is, a type is not copy- initializeable)

1. the init= function contains a compilerError call
2. there is an overload that can't be called say, where false, and it disables the generation of the default one
3. new syntax like proc init=(other) = none (or something)
4. use attributes on the type, with a generalized attribute syntax (see generalized attribute syntax #14141), to indicate it; e.g. @attribute(no copy init) record R { }

C1 compilerError

Pros:

• types can create a different error message e.g. "Can't copy a list with non-copyable elements"
• no new syntax is required

Cons:

• compiler has to do some special stuff for compilerErrors
• it's less obvious how this will work once we have interfaces

C2 where false

Pros:

• it arguably follows from B that this should work
• no new syntax is required

Cons:

• it might be a bit ugly / strange in practice. In particular, users will have to also put something in the body of this function. But what could it reasonably contain? Should it be empty?

C3 new syntax

Pros:

• new syntax can give us a general structure to provide pure virtual methods and opt-in to compiler-generated default initializer or init=
• while the new syntax might not come up often for users, it will be reasonably clear to read and won't contain things like superfluous method bodies

Cons:

• it requires new syntax
• perhaps pure virtual methods and opt-in to compiler-generated default initializer are different enough problems that they should have different solutions

C4 attributes

Pros:

• attribute can be reasonably clear to read and apply to type

Cons:

• what functions compiler generates might be too fundamental for attributes

[mppf]

There is some agreement on A and B.

For C, we will have C1 implemented now and hope to enable also C2 in this release.
We will leave C3/C4 as future work.

[mppf]

#15271 contains the remaining open questions. The other elements are addressed in PRs #14887 and #14956.

[mppf]

Issue #15838 is asking if we want to revisit the error if only one of = and init= is user provided strategy based upon experiences from users in issues #15717 and #15806.