Disallow rectangular arrays of non-nilable element types?

[vasslitvinov]

One way to address #13602 temporarily is to disallow arrays of non-nilable element types outright. This work was already started in #14846 and also discussed in #14367, where it applied to associative arrays and maps.

This issue is to discuss this for rectangular arrays.

One alternative to this approach is to allow a rectangular array of non-nilable elements when:

• it is initialized when it is declared (for variables), and
• its domain is constant or is a literal (so the array cannot be resized).

[vasslitvinov]

I implemented a blanket error for all arrays of non-nilable element types and got 370+ errors in a paratest with futures.

[ADDED] See my changes here.

Here are a couple of observations:

• Our distributions were hit -- every time a global descriptor class declares an array of local descriptor classes. This required a fair amount of added ! checks throughout the distribution code.

  While the amount of added ! is not too bad, I personally do not like that it obscures the fact that the elements of those arrays are always non-nilable.

  If we implemented the alternative I mention in the issue text, we can keep these arrays non-nilable. This will require switching the domain of "target locales" to const and doing away with assignment on _distribution aka dmap things. Brad, in a separate conversation, was OK with these constraints.

• The same applies to LCALS.

• The very first failed test in our test suite, analysis/alias/array-of-classes-variant, declares an array literal out of two non-nilable classes. We would need to edit our array-literal-creation code to make the type of the literal nilable in such a case.

• Our standard list datatype will have trouble supporting non-nilable elements -- only when the user invokes toArray() on it:

  proc const toArray(): [] eltType {
    var result: [1.._size] eltType;
  
    on this {
      _enter();
  
      var tmp: [1.._size] eltType;
  
      forall i in 1.._size do
        tmp[i] = _getRef(i);
  
      result = tmp;
  
  _leave();
    }
  
    return result;
  }

  I would propose to edit this method to switch the returned element type to be nilable if needed. If we implement my proposed alternative solution, we would still need to make this change, and I would propose to add another method, perhaps toArrayLocal, that does not have the on clause, allowing it to initialize result directly using a forall... _getRef expression.

[mppf]

Regarding array literals of non-nilable class type, I think we shouldn't worry about getting those working this release.

Regarding list and toArray, I think it would be OK for this to result in an error this release for non-nilable types. Longer-term, I don't think that we should add other variants of toArray or change the return type. With the alternative checking strategy, it will be possible to implement with e.g. var resultQ: [1.._size] eltType?; and then at the end return resultQ:class to cast away the nilability.

[bradcray]

Somewhat related: In PR #15187, I added an execution-time halt in the event that an array of non-nilable classes is resized. I think of this as a stopgap until we have a compile-time solution.

[bradcray]

I don't think this issue (completely disallow arrays of non-nilable) is really on the table anymore, and could probably be closed. @vasslitvinov, do you agree? We've got an execution-time check now to avoid resizing arrays of non-nilable, and issue #15094 as a placeholder for moving that check from execution-time to compile-time.

[vasslitvinov]

It is not on the table indeed. Closing.