Chapel: calling methods inside class/record init

[BryantLam]

Related #12371

Feature request? More of a brainstorming exercise that might lead somewhere. I maybe(?) want to do _something_ like the ability to call methods inside class/record init().

class C {
  const x, y: int; // const is important.
  proc init(a: int) {
    this.x = make_x(a);
    this.y = make_y(a); // y dependent on x.
    this.complete();
  }
  proc make_x(a: int) const: int { return a + 1; }
  proc make_y(a: int) const: int { return this.x + a + 2; }
}

This does not work today because of reasons that I think I know (and agree with), but someone else should probably say what these reasons actually are.

What I think I know.

1. The compiler likely has a harder time determining when a field is properly initialized before use if you allow the ability to call methods inside init before this.complete(), so method calls are not allowed for a simpler implementation.
2. The methods accept a this argument that is not properly typed during initialization (i.e., borrowed object instead of borrowed C).

Some alternative solutions. Any others?

1. Move the make functions to be outer/global functions instead of methods.
   Cons:
   (a) The global functions cannot take this as in make_y, so you have to spell out each argument that goes into making the output value, which might be numerous.
   (b) Functions are now visible outside the class when that initialization behavior is not intended to be visible. You can protect with private, but doesn't always help if e.g., a sibling item or a child module wants to do whatever.

2. Encapsulate inside init with another class/record that implements the initialization functions.
   Pros:
   (a) Even better encapsulation because it's no longer a method at all.
   Cons:
   (a) Boilerplate
   (b) Does not help detect when user inadvertently uses "uninitialized" field because the inner this.complete() will set all those fields to default values; e.g., calling make_y before make_x. There likely isn't a way around this behavior for any solution except the most strict.

class C {
  const x, y: int; // const is important.

  proc init(a: int) {
    record C_Init {
      var x, y: int;
      proc init(a: int) {
        this.complete(); // let me call methods.
        this.x = make_x(a);
        this.y = make_y(a); // y dependent on x.
      }
      proc make_x(a: int) const: int { return a + 1; }
      proc make_y(a: int) const: int { return this.x + a + 2; }
    }

    var c_init = new C_Init(a);
    this.x = c_init.x;
    this.y = c_init.y;
    this.complete();
  }
}

[bradcray]

This does not work today because of reasons that I think I know (and agree with), but someone else should probably say what these reasons actually are.

The reasons are that we wanted the language to guarantee that a user would not be sending an uninitialized, or partially initialized, object into a method that was expecting a complete object; or equivalently, we did not want the language definition to depend on the compiler's ability to determine that any given method was safe—was only using fields and object capabilities that had been initialized up until that point (e.g., make_x()'s reliance on nothing about this; make_y()'s reliance on x only); at least in this first draft of the language. More practically, we didn't want to make this into a full-fledged C until all of its fields had been initialized (and until it's a C, C methods can't be called on it).

[mppf]

@BryantLam - is there a reason you don't want to write this:

class C {
  const x, y: int; // const is important.
  proc init(a: int) {
    this.complete(); // stores 0 in the fields x and y
    this.x = make_x(a); // now OK, since it cannot access uninitialized memory
    this.y = make_y(a);
  }
  proc make_x(a: int) const: int { return a + 1; }
  proc make_y(a: int) const: int { return this.x + a + 2; }
}

?

[lydia-duncan]

I thought we'd written the justification for this somewhere? Or maybe we tried to but determined that the spec or wherever wasn't the right place to justify our decisions?

[BryantLam]

From @mppf's https://github.com/chapel-lang/chapel/issues/13171#issuecomment-498664449, I'd be open to that approach since init is a special function, so modifying const values during init-time may be okay, but I'll remark that I'm not certain since having init gain another special behavior may not be ideal either (though it is admittedly a minor special behavior with not a lot of mental burden).

C++, for example, has similar behavior to Chapel today if you try to modify a const variable during the body of the constructor instead of modifying it in the initialization list [1] which is analogous to modifying the const value before or after this.complete(), except we can't call methods before this.complete(). (Note: I'm not saying C++'s behavior is correct; it's just the same, except C++ can also call methods in the initialization list.)

A side remark is that noinit #8792 becomes more important if @mppf's programming pattern is to say "call your methods after this.complete()" since some of the data structures might be expensive to default initialize.

[mppf]

We did consider allowing const fields to be reset in Phase 2 (which today means after this.complete()). I think the main issue there is that the compiler will have to make different assumptions about the const-ness of those fields in the methods called in the initializer (like make_x) than it would for methods called only outside of the initializer.

[bradcray]

For those not following the last few comments (as I wasn't), Michael's example doesn't actually compile and run today because we don't currently permit const fields to be modified after this.complete() (because we treat them as already initialized, and const fields can't be re-assigned after initialization). So I interpret his question as being "Is there a reason you (Bryant) didn't request the ability to write it this way [to get similar behavior to C++]?" (where I'd originally interpreted the question to be "Why didn't you write this?")

It seems to me that one answer to Michael's question could be that the default initialization + reassignment approach could be unduly expensive for types that are larger / more complex than integers.

From a purity standpoint, I like that const fields can't be assigned after this.complete() because it feels like it really unifies initialization expressions and initialization via assignment in init() prior to this.complete(). I.e., I'd be reluctant to change this behavior. But the fact that C++ permits it is interesting and makes me feel less certain about that stance. [edited to correct my misinterpretation of what Bryant was saying]

One other approach that I've been kicking around (and which works today) is this:

class C {
  const x, y: int; // const is important.
  proc init(a: int) {
    this.x = make_x(a); // now OK, since it cannot access uninitialized memory
    this.y = make_y(a);

    inline proc make_x(a: int) const: int { return a + 1; }
    inline proc make_y(a: int) const: int { return this.x + a + 2; }
  }
}

The idea being to use local _nested_ procedures whose bodies would be legal initialization expressions to initialize the fields directly. The potential downside to this relates to why Bryant wants to create these as methods in the first place: If it's because he also wants to call them as methods on the final object, then it fails because their nested behavior makes them invisible outside of the class. But if it's really about creating longer code blocks to initialize things without blowing up the initializing expression slot, then maybe this is the answer.

(Also, note that the inline isn't strictly necessary, my head just went there before verifying that this worked since it seemed like "it'd be legal if I'd inlined it manually, so...")

[mppf]

Actually I just didn't initially notice that the fields were const. However I do think it's reasonable to consider allowing assignment to const fields in the body of the initializer after this.complete.

But the fact that C++ permits it is interesting and makes me feel less certain about that stance.

I thought @BryantLam was saying that C++ does not permit assigning to a const field .

I think the main issue there is that the compiler will have to make different assumptions about the const-ness of those fields in the methods called in the initializer (like make_x) than it would for methods called only outside of the initializer.

Correcting myself here - I don't think this is necessarily an issue. We could still assume that these fields are const in the methods - in the example I posted in particular this would be OK. It's just the initializer itself that would not assume that they are const (and it already doesn't, to initialize them in the first place).

[bradcray]

I thought @BryantLam was saying that C++ does not permit assigning to a const field .

Oh I see... I misunderstood his comment. OK, that makes me feel even better about where we are today then (sorry for my confusion).

[mppf]

I think D might allow re-assignment of const fields in the constructor...

[lydia-duncan]

Note that I wrote a few tests involving functions defined within initializers way back when (https://github.com/chapel-lang/chapel/tree/master/test/classes/initializers/phase1 + anything starting with "nested-function"). I suspect I only tossed the expected behavior past Mike Noakes and maybe Michael/Vass (but then again, maybe not the last two)

[bradcray]

Re-reading the original issue, I'm remembering that a con of using module-scope procedures was "Functions are now visible outside the class when that initialization behavior is not intended to be visible." So maybe this suggests that what I've done above is closer to an approach @BryantLam would like than I was guessing?

That said, I've also determined that the compiler isn't being smart about analyzing these nested procedures, it's just letting them pass through. So I could also create some that refer to the fields before initialization like:

    inline proc make_x(a: int) const: int { return /* should be illegal: */ this.y + a + 1; }

I'll open a bug issue on this soon.

[cassella]

6976 links to some of those test/classes/initializers/phase1/nested-function* futures. Though I think the futures focus on field reinitialization, not access to uninitialized fields.

[bradcray]

Thanks Paul! PR #13182 adds a test to that directory with similar naming showing that the "correct" idiom above works and a future showing that the incorrect idiom is also accepted by the compiler, but shouldn't be. I've added a mention of this future to your existing issue #6976.

[mppf]

Another potential way to write it is to use type methods. Still suffers from Con 1a from the original issue (you have to explicitly pass fields), but doesn't suffer from Con 1b.

class C {
  const x, y: int; // const is important.
  proc init(a: int) {
    this.x = make_x(a);
    this.y = make_y(this.x, a);
    this.complete();
  }
  proc type make_x(a: int) const: int { return a + 1; }
  proc type make_y(x: int, a: int) const: int { return x + a + 2; }
}

This uses type methods and passes the fields necessary directly.
It does not compile now (but maybe it should?).

This version works around the compilation error and appears to function today:

class C {
  const x, y: int; // const is important.
  proc init(a: int) {
    this.x = C.make_x(a);
    this.y = C.make_y(this.x, a);
    this.complete();
  }
  proc type make_x(a: int) const: int { return a + 1; }
  proc type make_y(x: int, a: int) const: int { return x + a + 2; }
}

var x = new C(1);

[BryantLam]

It does not compile now (but maybe it should?).

I don't think(?) it should compile. Calling directly with C.make_x() is how I'd expect it to work, but I also recognize that having good type alias support is important for class/records with lots of generic fields.

[BryantLam]

My intent was to make object initialization more abstracted, but I didn't want to expose those functions/methods outside of init. Brad's solution fits perfectly into that. Another use case that would rely more on methods would be setters/getters, but a setter on a const field doesn't make sense.

I'm satisfied with the solution that Brad provided https://github.com/chapel-lang/chapel/issues/13171#issuecomment-498800847. I'll save this thread as a design pattern. Thanks all!

[BryantLam]

Note that noinit is still important if one wanted to use setter/getter methods after this.complete().