dyno: incorrect error reported for task private variables using loop indices

[DanilaFe]

For the following piece of code:

  forall (idx1, (idx2, idx3)) in myIter() with (
    var tpv1= idx1,
    var tpv2= idx2,
    var tpv3: idx3.type
  ) {
    writeln((idx1, idx2, idx3, tpv1, tpv2, tpv3));
  }

The future .good files expect the following output:

indvar-errors-2.chpl:8: In function 'main':
indvar-errors-2.chpl:10: error: the initialization or type expression of the task-private variable 'tpv1' references the forall loop induction variable 'idx1'
indvar-errors-2.chpl:11: error: the initialization or type expression of the task-private variable 'tpv2' references the forall loop induction variable 'idx2'
indvar-errors-2.chpl:12: error: the initialization or type expression of the task-private variable 'tpv3' references the forall loop induction variable 'idx3'

The production compiler currently reports no errors at all. Dyno, on the other hand, reports:

indvar-errors-2.chpl:10: error: 'idx1' used before defined
indvar-errors-2.chpl:9: note: defined here
indvar-errors-2.chpl:11: error: 'idx2' used before defined
indvar-errors-2.chpl:9: note: defined here
indvar-errors-2.chpl:12: error: 'idx3' used before defined
indvar-errors-2.chpl:9: note: defined here

This is reported by the production compiler's passes after Dyno (i.e., Dyno is likely resolving idx1 etc. to the loop variables, and the production compiler, after rearranging some of the code to make the task-private variables work, notices the use-before-definition). So this behavior is incorrect, and should be fixed.

[bradcray]

So this behavior is incorrect, and should be fixed.

It seems like the new/Dyno errors are in the right spirit, though I could imagine them being confusing to an end-user. Specifically, task-private variables (tpvs) are created when the tasks are created while the idx# variables will be created within loops within each task, so calling this a use-before-def seems correct. It just may not be obvious to the user why this is a use-before def, especially given that the line numbers suggest they are being defined before (on an earlier line) they're used. We could view generating a more specialized error (like the one in the .good file) as being a specialization of the more general use-before-def error for tpvs.

Maybe this is what you're implying by "should be fixed" as well... I wasn't sure.

[DanilaFe]

Well, the trouble is that the "use after def" error is coincidental, I think. It's drawn from the passes following the Dyno scope resolver, not dyno itself. So more work needs to be done to make Dyno support the error natively, rather than by coincidence with the subsequent AST transformations.

[bradcray]

Does dyno already support use-after-def errors today? (I was guessing not, but am now realizing maybe I'm wrong about that?)

[mppf]

It does not, yet.

This program doesn't result in an error when given to `testInteractive`. ``` chapel proc foo() { x; var x = 23; } ```

Also, I don't think it's inherently obvious that the task-private variables are introduced into the same scope as the loop index variables. However that direction is completely fine with me.

[bradcray]

It does not, yet.

That makes the current dyno behavior seem like a positive step forward then: It is storing the AST in a way that we're now catching use-before-def errors that we didn't before. It doesn't seem as relevant to me whether dyno or the production compiler are generating those errors.

Also, I don't think it's inherently obvious that the task-private variables are introduced into the same scope as the loop index variables.

If forced to guess, I'd say they should be (or maybe "would typically be") introduced in an outer scope relative to the loop index variables. Which, now that you've pointed it out, I guess would suggest that "use before def" actually isn't an appropriate error... that it should be something more like "unresolved symbol"?