chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.75k stars 410 forks source link

dyno: resolve nested init calls #24906

Closed DanilaFe closed 3 weeks ago

DanilaFe commented 3 weeks ago

Closes https://github.com/chapel-lang/chapel/issues/24854. Closes https://github.com/Cray/chapel-private/issues/6182 (sprint task).

This PR adjusts the InitResolver to support calls like this.init inside other init procedures.

To make this work, the PR does the following things:

  1. It disallows calls to 'init' on anything other than 'super' and 'this'. As far as I know, this is already effectively the standard for Chapel; however, it was not enforced. As a result, the following program -- which is legal on main -- is now illegal.

    record myRec {
       var x: int;
    
       proc init(x: int, y: int) {
           this.x = x + y;
       }
    
       proc init(x: int) {
           init(x, 0);
       }
    
       proc init() {}
    }
    
    record container {
     var r: myRec;
    
     proc init() {
       init this;
       r.init(10); // bad bad bad
     }
    }
    
    var c: container;

    By doing so, the PR makes it possible for the InitResolver to syntactically detect calls such as this.init(). By doing this, in turn, we can start representing their effects on the resolution state.

  2. Adding new handling methods to InitResolver that operate on CallResolutionResults in addition to AST nodes. This is required because the effects of calling this.init depend on which overload of init is called; as such, we want to go through the usual mechanism for resolving the initializer. Doing so using CallResolutionResults makes the most sense. As a result of this PR, the handleCallExpr follows up resolution with handleResoledCall, which descends into the InitResolver.
    • When handling init, the InitResolver populates the individual field states as if they were assigned to. This is useful because the calls to 'this.init' need to fit into the existing framework of initialization: handling branches, merging intermediate results, etc.
    • This also requires a slight adjustment to the handleCallExpr to stop skipping call resolution when the receiver formal is generic. The this in this.init for a generic method is going to be generic, but that should not stop us from performing resolution.

As a result, https://github.com/chapel-lang/chapel/issues/24854 is resolved; a major consequence of that is that ranges with the by operator (mentioned in https://github.com/chapel-lang/chapel/pull/24853) now resolve correctly: the constructor that previously threw away the stride now properly takes it into account. I was able to validate that the implementation of ranges in Dyno works with strides as one would expect.

Reviewed by @benharsh -- thanks!

Testing