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

initializers and typeless or generic fields #16508

Closed
mppf opened this issue Sep 29, 2020 · 20 comments
Closed

initializers and typeless or generic fields #16508

mppf opened this issue Sep 29, 2020 · 20 comments

Comments

@mppf
Copy link
Member

mppf commented Sep 29, 2020

Follow-on to #12318 and issue #16044.

Issue #12318 discusses a language design direction that can handle typeless fields better.

We still have problems with the language design in this area. The main problems are around type aliases (and/or partial instantiations) and typeless fields.

For example:

record GenericTypedFieldWithInit {
  var f:numeric = 1;
  proc init() {
    this.f = 1.0;
  }
}
var x = new GenericTypedFieldWithInit(); // runs init()
var y: GenericTypedFieldWithInit; // should run init() but doesn't work yet on master
var z: GenericTypedFieldWithInit(int); // how can the `init` called respond to the fact that `f.type` is already `int` ?

According to #12318, initializer authors will use two features to be able to write an init that responds to the various possibilities:

  1. Using this.type to query elements of the type. This is already possible in init= functions but not yet in init
  2. A way to get the default value for a particular type. This is important for typeless fields cases.

However we currently have implemented a pass-type-alias-arguments-as-named-arguments-to-initializers strategy (see https://chapel-lang.org/docs/master/technotes/initTypeAlias.html). Does a user trying to write a proc init for GenericTypedFieldWithInit above need to create one with signature proc init(type f) for the compiler to pass the type of f into? That seems strange since the field f is not a type but rather a value. Should this information be passed exclusively through this.type - and in that event do we need to change something about the pass-type-alias-arguments-as-named-arguments-to-initializers strategy? Or, should the compiler try to call proc init(type fType) or something along those lines?

Update: My current understanding is that we can add this.type conveying the types for these typeless fields as a change that preserves compatability (i.e. is not a breaking change).

@mppf
Copy link
Member Author

mppf commented Sep 29, 2020

Here is more detail about the "doesn't work yet on master" comment:

record R {
  var x:numeric = 1;
  proc init() {
    this.x = 1.0;
  }
}

var a:R;
writeln(a);
writeln(isGenericType(a.type));
writeln(a, ": ", a.type:string, " field type ", a.x.type:string);
$ chpl rr.chpl 
$CHPL_HOME/modules/internal/ChapelIO.chpl:217: In function 'isIoField':
$CHPL_HOME/modules/internal/ChapelIO.chpl:218: error: the type of the actual argument 'call_tmp' is generic
note: generic actual arguments are not currently supported

I ran into this when working on #16030 and the pattern above is inspired by something we have currently on master in Treap.chpl.

@mppf
Copy link
Member Author

mppf commented Sep 29, 2020

It is appealing to imagine that the user could write this:

record GenericTypedFieldWithInit {
  var f:numeric = 1;
  proc init() {
    this.f = 1.0;
  }
  proc init(f:numeric) {
    this.f = 1;
  }
}

and then the compiler could compute the initialization expression 1 and pass it like init(f=1). However this strategy has complications with field initialization expressions that depend on previous fields. I'm not sure if those problems can be reasonably resolved or if we will need the user-provided initializer to accept the type information but not the default values.

mppf added a commit that referenced this issue Sep 30, 2020
Improve compiler's checking for resetting an instantiated field

This PR adjusts `resolveInitField` in function resolution to check if a
generic field already provided a type is being reset.  This provides a
good error message in cases such as described in issue #16030. In order
to get that to work, I also needed to update
`AggregateType::fieldIsGeneric`  and `AggregateType::generateType` to
better handle the case of a field with a generic type and an
initialization expression.

This PR adds a better error for the case in which a record field is
declared with a generic type and an initialization expression. In that
event, there are open language design questions about how to default
initialize a variable of that type. See #16508.

While there, this PR removes the `decayToBorrow` argument and
functionality from several `resolveGenericActuals` functions as it is no
longer necessary now that we can pass generic type arguments. It also
replaces a strcmp on astrs with a pointer comparison.

Resolves #16030.
Helps with, but does not fully resolve, #16044.

Reviewed by @e-kayrakli - thanks!

- [x] primers pass with verify+valgrind and do not leak
- [x] full local futures testing
@mppf mppf changed the title initializers and typeless fields initializers and typeless or generic fields Sep 30, 2020
@mppf
Copy link
Member Author

mppf commented Oct 1, 2020

@mppf
Copy link
Member Author

mppf commented Nov 3, 2020

My current thinking is:

1. A Bug

There is a bug in that code like this

record TypelessField {
  var field;
  proc init(...) { ... }
}
type TypelessFieldInt = TypelessField(int);
var b: TypelessFieldInit;

in that the compiler tries to translate it into

var defaultTmp: int;
var b = new TypelessFieldInt(field=defaultTmp);

2. How to handle typeless fields

The compiler will continue to use named arguments for type and param fields. But, for generic (value) fields it will use this.type to convey the type information.

Note to self - I made a slide deck about this for the 1.23 release notes that we decided not to include. But it presents the above two points (as well as alternative designs).

@mppf mppf self-assigned this Jan 22, 2021
mppf added a commit to mppf/chapel that referenced this issue Jan 26, 2021
See issue chapel-lang#16508. In the future we would like 'this.type'
to convey the type information for a typeless field as in

record TypelessField {
  var field;
  proc init() { /* this.type.field indicates int in the below case */ }
}
type TypelessFieldInt = TypelessField(int);
var b: TypelessFieldInit;

---
Signed-off-by: Michael Ferguson <[email protected]>
mppf added a commit to mppf/chapel that referenced this issue Jan 26, 2021
See issue chapel-lang#16508. In the future we would like 'this.type'
to convey the type information for a typeless field as in

record TypelessField {
  var field;
  proc init() { /* this.type.field indicates int in the below case */ }
}
type TypelessFieldInt = TypelessField(int);
var b: TypelessFieldInit;

---
Signed-off-by: Michael Ferguson <[email protected]>
mppf added a commit that referenced this issue Jan 27, 2021
Prohibit default initialization for records with typeless fields and user initializers

This PR adjusts the compiler so that the bug described in #16508 emits a
compilation error instead of unexpected behavior. The goal here is to
allow further improvements to address #16508 to happen without breaking
existing code that compiles.

In particular, for this code:

``` chapel
record TypelessField {
  var field;
  proc init() {
    field = 1.0;
    writeln("init()");
  }
  proc init(field) {
    this.field = field;
    writeln("init(field)");
  }

}
type TypelessFieldInt = TypelessField(int);
var b: TypelessFieldInt;
writeln(b.field, ": ", b.field.type:string);
```

Before this PR, the compiler would rewrite the default initialization of
`b` as:

``` chapel
var defaultTmp: int;
var b = new TypelessFieldInt(field=defaultTmp);
```

(As a result, it would compile before this PR and print "init(field)").

However this is surprising (Shouldn't `init()` be called?) and counter to
the design of initializers (because the record author doesn't have an
opportunity to control the default value of `field` in the default
initialization).

Instead of calling "init(field)", we expect this program to fail to
compile because the default initialization would run `init()` and
encounter a type mismatch. To get that working nicely will require using
`this.type` to convey the type information in this case (see also #16508).
This PR puts off that effort in favor of prohibiting the pattern that would
change in behavior.

In particular, this PR makes it an error "this default-initialization is
not yet supported". This error is demonstrated in the new test
issue-16508-user.chpl.

This PR allows types that use the compiler-generated default initializer
to continue to use the default value for the field type when
default-initializing since that does not present stability problems with
the change planned in #16508. This is demonstrated in the new test,
issue-16508-default-ok.chpl.

Besides adding the error, this PR uses a combination of adding
workarounds and updating tests to be futures. I added workarounds for
tests that seemed to have a goal different from checking this pattern.
Also note that reuse-init-instantiation-var.chpl was the same as
reuse-init-new-instantiation-var.chpl so this PR removes it.

- [x] full local futures testing

Reviewed by @lydia-duncan - thanks!
@e-kayrakli
Copy link
Contributor

Potentially related: #18664

@mppf
Copy link
Member Author

mppf commented Mar 28, 2022

Following up on - #16508 (comment) - I have some discomfort that the design proposed there spans two different approaches (one for init= using this.type and one for init using named arguments). Not sure how to fix that without making breaking changes.

@mppf
Copy link
Member Author

mppf commented Nov 22, 2022

Regarding the comment #16508 (comment) -- the "bug" is what makes this example compile and run:

record R {
  var field;
}

var x: R(int); // compiles and runs by effectively calling R.init(0)
writeln(x);

But,

record R {
  var field;
  // What signature to write if I want a custom default initializer?
  proc init(type field) {
    var tmp: field;
    this.field = tmp;
  }
}

var x: R(int); // fails to compile
writeln(x);

This second example gives an error about it not being supported yet:

bb.chpl:10: error: default initialization with type 'R(int(64))' is not yet supported
bb.chpl:2: note: field 'field' is a generic value
bb.chpl:2: note: consider separately declaring a type field for it or using a 'new' call

Here is another example of trying to write a custom default initializer that gives a similar error:

record R {
  var field;
  // What signature to write if I want a custom initializer?
  proc init() {
    var tmp: this.type.field;
    this.field = tmp;
  }
}

var x: R(int);
writeln(x);

@vasslitvinov
Copy link
Member

I propose that, in the context of the previous comment, we switch from calling R.init(0) to calling proc R.init(type field) {...} for var x: R(int);

The compiler would generate these initializers, or they can be provided by the user:

  • proc R.init(type field) to invoke for var x: R(someType);
  • proc R.init(in field) to invoke for new R(someVal)
  • init= (and others?) as usual

Why not R.init() that looks at this.type.field?

Such an initializer could potentially be used instead of R.init(type field). The reason to prefer the latter is the uniform treatment of untyped and concretely-typed fields. For example, if the record author switches a field from untyped to concretely-typed or visa versa, the user-defined R.init() would need to adjust whereas R.init(type field) would not. Also, the body of a simple user-defined initializer will look uniform under the latter approach and not under the former, when R contains a mix of the two kinds of fields.

Granted, seeing 'field' with a type intent in one of the initializers looks a bit odd. However I suggest that this is a small price to pay for this functionality. It does not push me towards the R.init() approach.

What about the combinatorial explosion?

Given a record with multiple untyped fields, it is natural to support "combo" construction of an R instance, where some fields are default-initialized based on their type and the others are given initial values.

record R {
  var field1, ..., fieldN;
}

// assume set(field1, ..., fieldN) is partitioned info
// set(fieldA, ..., fieldC) and set(fieldX, ..., fieldZ)

type T = R(fieldA = typeA, ..., fieldC = typeC);
var r = new T(fieldX = valX, ..., fieldZ = valZ);

Supporting combo construction looks challenging because there is a combinatorial number of partitions of set(field1, ..., fieldN) and each partition requires its own "combo" initializer of the form:

// formals to be reordered in the field declaration order
proc R.init(type fieldA, ..., type fieldC,
            in fieldX, ..., in fieldZ)
{...}

I propose to address this challenge as follows.

  • The language supports combo construction.
  • The user is welcome to provide combo initializers for the partition(s) of interest to them, possibly for all partitions.
  • In the absence of user-defined initializers, our current implementation produces a "not implemented" error when combo construction is attempted. Or...
  • Alternatively, the implementation creates combo initializers on demand, i.e., only for those partitions that have combo 'new' calls in the code being compiled.

@mppf @benharsh fyi; related to Approach 5 in #19120

@mppf
Copy link
Member Author

mppf commented Jan 11, 2023

Why not R.init() that looks at this.type.field?

... For example, if the record author switches a field from untyped to concretely-typed or visa versa, the user-defined R.init() would need to adjust whereas R.init(type field) would not.

That specific statement does not make sense to me. If the field is switched from untyped to concretely typed, then the function called for default initialization will change from R.init(type field) to R.init(). That implies to me that the user would have to change the signature of this method, and so would necessarily need to adjust it.

What about the combinatorial explosion?

The user is welcome to provide combo initializers for the partition(s) of interest to them, possibly for all partitions.

I don't think we should do that because it creates busy-work for users who want to make their types as flexible as possible.

I also wanted to add that the combinatorial explosion problem also applies when partial instantiation is used.

Lastly, I was wondering about the situation with the example record we discussed yesterday, since it appears that the type constructor does use fieldName=fieldType for fields like var fieldName.

record R {
  type T;      // ‘type’ field  
  param p;     // ‘param’ field  
  var v;       // untyped ‘var’ or ‘const’ field  
  var i: integral;  // a ‘var’ or ‘const’ field  
                    // whose type is generic  
  var j: int;  // cf. this does not make it generic  
}

type t = R(v=bool);

var x = new t(T=int, p=4, i=32, j=5);
// error: A type alias of 'R' may not be used in a new-expression because it contains a typeless field ('v')

// this does work, but the compiler is computing the default values
// and passing them to the initializer, as discussed above
type tt = R(T=int, p=3, v=real, i=uint);
var y: tt;

@vasslitvinov
Copy link
Member

The user is welcome to provide combo initializers for the partition(s) of interest to them, possibly for all partitions.

I don't think we should do that because it creates busy-work for users who want to make their types as flexible as possible.

In this case, perhaps the approach we have today where the compiler fills in the default values is good?

The downside is that it does not allow the initializer to discern whether the actual argument was a compiler-provided default or a user-supplied value. So perhaps we can make this scheme an opt-in?

If the field is switched from untyped to concretely typed, then the function called for default initialization will change from R.init(type field) to R.init().

Good point. I need to think more about it.

@mppf
Copy link
Member Author

mppf commented Jan 11, 2023

The downside is that it does not allow the initializer to discern whether the actual argument was a compiler-provided default or a user-supplied value. So perhaps we can make this scheme an opt-in?

The user should be able to provide the default value by writing their own initializer (with some signature). The approach today doesn't have that property.

@bradcray
Copy link
Member

it does not allow the initializer to discern whether the actual argument was a compiler-provided default or a user-supplied value

Probably just an aside, but if it matters to this conversation, there's been a long-term desire to add language support for distinguishing between these cases (e.g., "In proc foo(x=42) is x 42 because the user didn't supply x or because they did and they supplied a value of 42?").

@bradcray
Copy link
Member

Something I was musing about this week w.r.t. the challenge case in #16508 (comment) was whether when the language sees:

record R {
  var field;
}

it should no longer create a type constructor/initializer with the signature proc R.init(type _fieldType) type to indicate the type of the field field; but instead, that it should only permit the type constructor R(?). Essentially, the author of the record has not provided a way to specify the type of the field, so a user should not be expected to be able to write it using R(int). Thus, if a user did write:

var x: R(int);

they would get an error like "R(int) is not a legal type signature" (maybe w/ the potential bonus "only R(?) is legal", though that may be hard to get right for a larger type). And the user would have to re-write their code as something like:

var x: R(?);
x = new R(42);

if they wanted to specify a type signature and do split initialization. Basically, R does not have a default type because it's too generic and it doesn't have a type signature because it's too vague so this is the best a user can do if they want to annotate the type of x. For a record like:

record R2 {
  type t;
  param p: int;
  var x;
}

type signatures like R2(int, 3, ?) or R2(p=3, ?) would be legal partial instantiations, but there would still be no way to create a complete/concrete type.

While the "implicit type field for the type-less variable" idea that we currently have is cute and seemed sensible, it's also subtle. And if it leads to weirdness like we're seeing, it doesn't seem worth preserving. In contrast, this proposal has the benefit of reflecting more directly/simply what the type's fields are, and puts the burden back on the type author if they want to support a complete/concrete type.

The only drawback I can think of is whether users would trip over this more often than desired/intended. For example, if I wrote:

record R {
  var c: MyConcreteClass;  // /generic class management
  var c2: MyGenericClass(?);  // + generic class
  var a: [] int;  // no idea what kind of domain map this array is
}

it seems the presence of such fields, being generic, would also result in my needing to type R(?). And maybe that's just fine, just pointing it out.

@mppf
Copy link
Member Author

mppf commented Jan 30, 2023

@bradcray - I interpret your proposal as "Some features don't work with typeless fields" which would be fine with me. To me, this not that different in the solution space from issuing a warning for fields like var x;; however I can appreciate that you see benefits from having it instead be error messaging that is more connected to what you are trying to do with the type.

Another thing I appreciate about your proposal is that it does not require us to solve user-defined type constructors to make progress on this specific issue. So, I support your proposal. Of course, we will need to do some testing to understand whether or not we are underestimating the impact on user code / realistic code.

A nit:

it should no longer create a type constructor/initializer with the signature proc R.init(type _fieldType) type to indicate the type of the field field;

That is not what we have today. Today, the type argument for the type constructor matches the field name:

record genericRecord {
  var field;
}

// default-initialize with field=int
var x: genericRecord(int);
writeln(x , " ", x.type:string);

// using named-argument passing works too, with the name 'field':
var y: genericRecord(field=int);
writeln(y , " ", y.type:string);

That said:

While the "implicit type field for the type-less variable" idea that we currently have is cute and seemed sensible, it's also subtle. And if it leads to weirdness like we're seeing, it doesn't seem worth preserving.

I think the idea is subtle whether the type constructor is proc genericRecord(type field) or proc genericRecord(type fieldType). Arguably the 1st of these is less subtle, but maybe it is just subtle in different ways.

@mppf
Copy link
Member Author

mppf commented Apr 13, 2023

@jhh67 brought up the idea that one could use x.type as a formal initializer argument name:

record R {
  var x;
}
var c: R(int);
// calls R.init(x.type=int)

// so you could write

proc R.init(type x.type) {
  x = 4; // or whatever makes sense
}

@mppf
Copy link
Member Author

mppf commented Jul 28, 2023

I've been looking at not allowing type construction for records like record R { var x; }, but it doesn't look like a good idea to me -- see #21993 (comment) for details.

What could we do instead? I can think of two possible directions. With either of these strategies, I think we could, in the future, add support for this.type within initializers, as a non-breaking change. So, I will leave that part alone for now.

Before getting into the two possible directions, I'd like to restate, what problem are we solving here?

Restatement of the Problems

I think that there are two problems.

Not possible to write a custom initializer to respond to an instantiated type with typeless field

For example, we might try to write it this way:

record U {
  var genericField;
  proc init(type genericField) {
    var tmp: genericField; // default initialize
    this.genericField = tmp;
  }
  proc init(in genericField) {
    this.genericField = genericField;
  }
}

var x: U(int); // error here, currently
writeln(x, " : ", x.type:string);

However, this does not compile. It gives

error: default initialization with type 'U(int(64))' is not yet supported
note: field 'genericField' is a generic value
note: consider separately declaring a type field for it or using a 'new' call

Default initialization for the compiler-generated initializer is worrying

Given that the above does not work, it seems worrying that the same pattern does work with the default initializer:

record R { var genericField; }

var x: R(int); // ok
writeln(x, " : ", x.type:string);

The compiler generated initializer for this record has the signature proc R.init(in genericField). But, we have the strategy of passing a type argument genericField=int for initializing such instantiations. So, when the compiler is faced with having a type actual for a compiler-generated-default initializer in formal, the compiler default-initializes (to convert the type into a value). This seems worryingly inconsistent with the above case with a user-provided initializer, and additionally it seems worrying in that it adds a default initialization that might not be obvious.

Potential Paths

Do not allow default initialization of such types for now

Currently, default initialization is not allowed for such types if there is a user-provided initializer. We could extend this restriction to types with compiler-generated initializers. That would make the two cases consistent.

If this is not allowed for now, then in the future, we can allow this only for user-defined initializers that meet certain constraints. In particular, we might require use of this.type or perhaps we look specifically for initializers to which we can pass type arguments, e.g. proc R.init(type genericField). One drawback of the the approach with formals like type genericField is that it can lead to a combinatorial explosion if a type wishes to support all of the combinations of types and values (since each such field could be represented with a type or in formal, and in principle, any combination of these might be used, with partial instantiations and a new call). However, maybe that is not such a big deal, since the most common use cases are type construction (to fully instantiate); calling new on the generic type; or calling new on a fully instantiated type.

Enable the default initialization for user-provided initializaters

As a convenient way to address the combinatorial explosion, we could extend the current behavior to user-provided initializers, under certain circumstances. For example:

record U {
  var genericField;
  proc init(in genericField) {
    this.genericField = genericField;
  }
}

var x: U(int); // this could default initialize an 'int' and pass it as the value for the initializer's 'genericField' formal
writeln(x, " : ", x.type:string);

This could be really convenient (since it's hard to imagine what else a user-provided initializer would do in such a case) but it has two drawbacks:

  1. It doesn't directly make it possible for the user to define an initializer that indicates what the default values should be. For example, a user might want the int field to have a default value of 1. I would expect that the compiler would only use the default-initialization strategy if the proc U.init(type genericField) was not available; and so that type formal version could be used to control the default values. TBD how the compiler could appropriately detect this situation when faced with multiple fields.

  2. It is subtle and a new case in which default initialization is used that might not be obvious to Chapel programmers.

@mppf
Copy link
Member Author

mppf commented Jul 28, 2023

Do not allow default initialization of such types for now

I've created a PR to explore this option: #22836.

@bradcray
Copy link
Member

bradcray commented Aug 8, 2023

I'm now caught up on this latest comment, but am not sure I fully understand everything about it (and may benefit from an interactive discussion). Thanks for starting by recapping the problems, and I haven't taken the time to re-read the whole thread, so apologize if I'm repeating earlier comments or questions as a result (this issue refuses to stick in my brain). I'll type my questions/thoughts/notes here, though you're welcome to reply in person if it's more efficient.

In your first example:

record U {
  var genericField;
  proc init(type genericField) {
    var tmp: genericField; // default initialize
    this.genericField = tmp;
  }
  proc init(in genericField) {
    this.genericField = genericField;
  }
}

var x: U(int); // error here, currently
writeln(x, " : ", x.type:string);

what would we expect/hope that this code would do? Guessing at the answer:

  • since we don't have user-defined type-constructors (yet), the only type constructor this declaration could call is the compiler-generated one (i.e., : U(int) would not call the first of the two initializers, but the compiler's type constructor, which is effectively something like this: proc type U.init(type genericField) { this.genericField.type = genericField; })
  • then, since it isn't initialized by the user (and is used before an assignment), the compiler would look for a 0-argument initializer, not find one, and error as it is.

So the current behavior seems reasonable to me (and I think you're saying that it's not unreasonable, just that it's not aligned with the all-compiler-generated behavior / doesn't support the user's ability to write similar code to the compiler?). So arguably the problem is that if we try to write the 0-argument initializer, we might try this:

  proc init(in genericField = defaultOf(this.genericField.type)) {
    this.genericField = genericField;
  }

but this doesn't work because the argument list can't refer to this; and if we try like this:

  proc init() {
    var tmp: this.genericField.type;
    this.genericField = tmp;
  }

then we're trying to refer to a field before it's declared. And if we do this:

  proc init() {
    var tmp: this.type.genericField;
    this.genericField = tmp;
  }

then we have the problem that we can't currently refer to this.type in initializers today that you mention above? (though, interestingly to me, the error looks identical to the original code which is intriguing b/c the original error made me think "sure, of course", whereas this one makes me ask "why not?" Whereas I was expecting something more like a complaint about being unable to refer to this.type.

Interestingly, changing the declaration to var x: U(int) = new U(); causes it to call into the 0-argument initializer and start complaining about it... which makes it look like it can't distinguish that U.genericField should refer to the type aspect of genericField rather than the value aspect?)

Actually, maybe that's my only [series of] questions...?

@mppf
Copy link
Member Author

mppf commented Aug 8, 2023

  • since we don't have user-defined type-constructors (yet), the only type constructor this declaration could call is the compiler-generated one (i.e., : U(int) would not call the first of the two initializers, but the compiler's type constructor, which is effectively something like this: proc type U.init(type genericField) { this.genericField.type = genericField; })

Right, but I don't think the type construction process is particularly relevant here.

  • then, since it isn't initialized by the user (and is used before an assignment), the compiler would look for a 0-argument initializer, not find one, and error as it is.

I see it differently.

Today, we have this strategy that we use named expressions to pass the type/param components of an instantiated type to the proc init call in order to default initialize. See also https://chapel-lang.org/docs/technotes/initTypeAlias.html . So, that means, even for a record with a type field, we need a 1-argument initializer to default initialize. For example:

record R {
  type T;
  var x : T;
}

var x: R(int);
//   compiler translates it into var x = new R(T=int)

The technote linked above says:

Fully-generic fields, like var x;, present a design challenge when using a type alias in a new-expression. The compiler-generated initializer, and most user-defined initializers, expect a value - not a type. It remains an open question as to how the compiler should invoke such initializers.

This is what we are facing in this issue. IMO if we turn off the special behavior for the compiler-generated default initializer, we can create an additive / non-breaking change in the future to enable user-defined initializers in such cases to accept the type. I think the most obvious strategy (given our other language design choices) would be for the signature for such an initializer to be what I showed above

record U {
  var genericField;
  proc init(type genericField) { ... } // describe how to default initialize R(int) etc
}

I think we should consider other options as well (including having the compiler try to default-initialze and pass to a proc init(in genericField) in some cases). The only direction I know of that would be breaking would be if we entirely abandoned the named arguments strategy described in the technote and instead went with using this.type for init as well as for init=. However, IMO it is not worthwhile to make such a big change to resolve this relatively small problem.

mppf added a commit that referenced this issue Aug 28, 2023
For #16508.

This PR changes the behavior of examples like the below, when the type
uses a compiler-generated default initializer:

``` chapel
record R { var genericField; }

var x: R(int); // was OK; now error

record U { var genericField: integral; }

var x: U(int); // was OK; now error
```

Historically, when faced with a default initialization like `var x:
R(int)` above, the compiler would default initialize an `int` to pass as
the value for `genericField`. However, it is worrying that this pattern
default-initializes something without it being clear that the user has
requested it. A further concern is that the default initializer should
be possible for users to write, but this is a case where a user cannot
write it. Additionally, a similar pattern was already producing a
compilation error if the type has user-provided initializers. That makes
sense, because it would be reasonable for a type author to assume that
they could customize the default value of such a field, when in fact
today, they cannot.

This PR changes the compiler to emit an error for default initialization
of a record using the compiler-generated initializer a typeless or
generic value field. The new error does not apply to `type` or `param`
fields. Note that such patterns already generated an error when the type
did not rely on the compiler-generated initializer. So, this PR makes
this error apply also to the case of compiler-generated initializers.

Reviewed by @vasslitvinov - thanks!

- [x] full comm=none testing
@mppf
Copy link
Member Author

mppf commented Aug 28, 2023

Closing. PR #22836 removed the special behavior for default initialization with var x; or var y: integral; style fields when using the compiler-generated initializer. It is not currently possible to get default initialization with record types containing such fields fields. While we can consider supporting it in the future, that is best viewed as a feature request. We can keep issue #16044 open for that feature request & I'll add a comment there summarizing the current situation.

@mppf mppf closed this as completed Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants