Deprectae ref-maybe-const task intents (#23049)

Deprecates ref-maybe-const for task intents. The compiler will warn when
a ref-maybe-const task intent is inferred to be `ref` in
`cullOverReferences`.

Implements part of the decisions for
#21398 (comment)

Followup to #22958

## Summary of changes
- enable warning for task intents
- add extra logic to handle task variables for record method receivers
and nested functions.
- add explicit task variables where needed in modules and tests on
`coforall` and `[co]begin`

## New tests
- add `test/deprecated/ref-maybe-const-task-intents.chpl`

## Testing
- paratest with futures
- paratest with gasnet

[Reviewed by @vasslitvinov]

closes Cray/chapel-private#5253

compiler/resolution/cullOverReferences.cpp

@@ -467,22 +467,22 @@ static void maybeIssueRefMaybeConstWarning(ArgSymbol* arg) {
   bool fromPragma = arg->hasFlag(FLAG_INTENT_REF_MAYBE_CONST_FORMAL);
 
   bool isCompilerGenerated = false;
-
   bool isTaskIntent = false;
   if (FnSymbol* fn = arg->getFunction()) {
     isTaskIntent = fn->hasEitherFlag(FLAG_COBEGIN_OR_COFORALL, FLAG_BEGIN);
-
     isCompilerGenerated = fn->hasFlag(FLAG_COMPILER_GENERATED);
   }
-
   // we have full control here, but this should be ok
   bool isOuter = arg->hasFlag(FLAG_OUTER_VARIABLE);
 
-  bool notImplementedYetOptOut = isArgThis || isTaskIntent;
-
-
+  bool notImplementedYetOptOut = isArgThis;
   bool shouldWarn = !notImplementedYetOptOut && !isOuter && !fromPragma && !isCompilerGenerated;
 
+  // if its an outer variable but its used in a task intent, warn
+  if (!shouldWarn && isOuter && isTaskIntent) {
+    shouldWarn = true;
+  }
+
   if (shouldWarn) {
     IntentTag defaultIntent = blankIntentForType(arg->type);
     // if default intent is not ref-maybe-const, do nothing
@@ -493,19 +493,32 @@ static void maybeIssueRefMaybeConstWarning(ArgSymbol* arg) {
 
     const char* argName = nullptr;
     char argBuffer[64];
-    if (!arg->hasFlag(FLAG_EXPANDED_VARARGS)) {
-      argName = arg->name;
-    } else {
+    if (isTaskIntent && arg->hasFlag(FLAG_FIELD_ACCESSOR)) {
+      sprintf(argBuffer, "this");
+      argName = argBuffer;
+    } else if (arg->hasFlag(FLAG_EXPANDED_VARARGS)) {
       int varArgNum;
       int ret = sscanf(arg->name, "_e%d_%63s", &varArgNum, argBuffer);
       CHPL_ASSERT(ret == 2);
       argName = argBuffer;
+    } else {
+      argName = arg->name;
     }
 
-    USR_WARN(arg,
-              "inferring a default intent to be 'ref' is deprecated "
-              "- please use an explicit intent for the argument '%s'",
-              argName);
+    const char* intentName = isTaskIntent ?
+      "add an explicit 'ref' task intent for" :
+        (isArgThis ?
+          "use an explicit 'ref' this intent for the method" :
+          "use an explicit 'ref' intent for the argument");
+
+    bool useFunctionForWarning = isTaskIntent && arg->getFunction();
+    Symbol* warnSym =
+      useFunctionForWarning ? (Symbol*)arg->getFunction() : (Symbol*)arg;
+    USR_WARN(warnSym,
+            "inferring a default intent to be 'ref' is deprecated "
+            "- please %s '%s'",
+            intentName,
+            argName);
   }
 }
 

compiler/resolution/implementForallIntents.cpp

@@ -1083,6 +1083,7 @@ static ArgSymbol* createArgForFieldAccess(ArgSymbol* thisArg, FnSymbol* fn,
   IntentTag intent = isConst ? INTENT_CONST : INTENT_BLANK;
   ArgSymbol* fieldArg = new ArgSymbol(intent, astr(fieldSym->name, "_arg"),
                                       fieldSym->type);
+  fieldArg->addFlag(FLAG_FIELD_ACCESSOR);
   fn->insertFormalAtTail(fieldArg);
 
   // Pass an actual correspondingly.

modules/dists/BlockCycDist.chpl

@@ -225,7 +225,7 @@ class BlockCyclic : BaseDist {
     const dummyLBC = new unmanaged LocBlockCyclic(rank, idxType, dummy=true);
     var locDistTemp: [targetLocDom] unmanaged LocBlockCyclic(rank, idxType) = dummyLBC;
 
-    coforall locid in targetLocDom do
+    coforall locid in targetLocDom with (ref locDistTemp) do
       on this.targetLocales(locid) do
         locDistTemp(locid) = new unmanaged LocBlockCyclic(rank, idxType, locid, this.lowIdx, this.blocksize, targetLocDom);
 
@@ -596,7 +596,7 @@ proc BlockCyclicDom.dsiBuildArray(type eltType, param initElts:bool) {
   const creationLocale = here;
 
   // formerly BlockCyclicArr.setup()
-  coforall localeIdx in dom.dist.targetLocDom with (ref myLocArrTemp) {
+  coforall localeIdx in dom.dist.targetLocDom with (ref locArrTemp, ref myLocArrTemp) {
     on dom.dist.targetLocales(localeIdx) {
       const LBCA = new unmanaged LocBlockCyclicArr(eltType, rank,
                                                    idxType, strides,

modules/dists/BlockDist.chpl

@@ -2006,7 +2006,7 @@ proc BlockArr.doiScan(op, dom) where (rank == 1) &&
   const sameDynamicDist = sameStaticDist && dsiDynamicFastFollowCheck(res);
 
   // Fire up tasks per participating locale
-  coforall locid in targetLocs.domain {
+  coforall locid in targetLocs.domain with (ref res, ref elemPerLoc, ref outputReady) {
     on targetLocs[locid] {
       const myop = op.clone(); // this will be deleted by doiScan()
 

modules/dists/CyclicDist.chpl

@@ -334,7 +334,7 @@ class CyclicImpl: BaseDist {
     const dummyLC = new unmanaged LocCyclic(rank, idxType, dummy=true);
     var locDistTemp: [targetLocDom] unmanaged LocCyclic(rank, idxType)
           = dummyLC;
-    coforall locid in targetLocDom do
+    coforall locid in targetLocDom with (ref locDistTemp) do
       on targetLocs(locid) do
        locDistTemp(locid) =
          new unmanaged LocCyclic(rank, idxType, locid, startIdxTemp, ranges);
@@ -470,7 +470,7 @@ override proc CyclicImpl.dsiNewRectangularDom(param rank: int, type idxType, par
   const dummyLCD = new unmanaged LocCyclicDom(rank, idxType);
   var locDomsTemp: [this.targetLocDom] unmanaged LocCyclicDom(rank, idxType)
         = dummyLCD;
-  coforall localeIdx in this.targetLocDom do
+  coforall localeIdx in this.targetLocDom with (ref locDomsTemp) do
     on this.targetLocs(localeIdx) do
       locDomsTemp(localeIdx) = new unmanaged LocCyclicDom(rank, idxType,
              this.getChunk(whole, localeIdx): myBlockType(rank, idxType));
@@ -639,7 +639,7 @@ proc CyclicDom.dsiBuildArray(type eltType, param initElts:bool) {
   var myLocArrTemp: unmanaged LocCyclicArr(eltType, rank, idxType)?;
 
   // formerly in CyclicArr.setup()
-  coforall localeIdx in dom.dist.targetLocDom with (ref myLocArrTemp) {
+  coforall localeIdx in dom.dist.targetLocDom with (ref locArrTemp, ref myLocArrTemp) {
     on dom.dist.targetLocs(localeIdx) {
       const LCA = new unmanaged LocCyclicArr(eltType, rank, idxType,
                                              dom.locDoms(localeIdx),

modules/dists/StencilDist.chpl

@@ -573,7 +573,7 @@ proc StencilImpl.init(boundingBox: domain,
   const dummyLS = new unmanaged LocStencil(rank, idxType, dummy=true);
   var locDistTemp: [targetLocDom] unmanaged LocStencil(rank, idxType) = dummyLS;
 
-  coforall locid in targetLocDom do
+  coforall locid in targetLocDom with (ref locDistTemp) do
     on this.targetLocales(locid) do
       locDistTemp(locid) = new unmanaged LocStencil(rank, idxType, locid,
                                                     boundingBox, targetLocDom);
@@ -669,7 +669,7 @@ override proc StencilImpl.dsiNewRectangularDom(param rank: int, type idxType,
   const dummyLSD = new unmanaged LocStencilDom(rank, idxType, strides);
   var locDomsTemp: [this.targetLocDom]
                   unmanaged LocStencilDom(rank, idxType, strides) = dummyLSD;
-  coforall localeIdx in this.targetLocDom do
+  coforall localeIdx in this.targetLocDom with (ref locDomsTemp) do
     on this.targetLocales(localeIdx) do
       locDomsTemp(localeIdx) =
         new unmanaged LocStencilDom(rank, idxType, strides,
@@ -947,7 +947,7 @@ proc StencilDom.dsiBuildArray(type eltType, param initElts:bool) {
   var myLocArrTemp: unmanaged LocStencilArr(eltType, rank, idxType, strides)?;
 
   // formerly in StencilArr.setup()
-  coforall localeIdx in dom.dist.targetLocDom with (ref myLocArrTemp) {
+  coforall localeIdx in dom.dist.targetLocDom with (ref locArrTemp, ref myLocArrTemp) {
     on dom.dist.targetLocales(localeIdx) {
       const LSA = new unmanaged LocStencilArr(eltType, rank, idxType, strides,
                                               dom.getLocDom(localeIdx),

modules/internal/DefaultRectangular.chpl

@@ -2478,7 +2478,7 @@ module DefaultRectangular {
     var state: [rngs.domain] resType;
 
     // Take first pass over data doing per-chunk scans
-    coforall tid in rngs.domain {
+    coforall tid in rngs.domain with (ref state) {
       const current: resType;
       const myop = op.clone();
       for i in rngs[tid] {

modules/packages/DistributedBag.chpl

@@ -1176,7 +1176,7 @@ module DistributedBag {
                     segment.releaseStatus();
                     coforall segmentIdx in 0..#here.maxTaskPar {
                       var stolenWork : [{0..#numLocales}] (int, c_ptr(eltType));
-                      coforall loc in parentHandle.targetLocalesNotHere() {
+                      coforall loc in parentHandle.targetLocalesNotHere() with (ref stolenWork) {
                         if loc != here then on loc {
                           // As we jumped to the target node, 'localBag' returns
                           // the target's bag that we are attempting to steal from.

modules/packages/LinearAlgebra.chpl

@@ -985,15 +985,15 @@ proc inner(const ref A: [?Adom] ?eltType, const ref B: [?Bdom]) {
 
     var localResults: [Locales.domain] eltType = 0;
 
-    coforall l in Locales do on l {
+    coforall l in Locales with (ref localResults) do on l {
       const maxThreads = if dataParTasksPerLocale==0
                          then here.maxTaskPar else dataParTasksPerLocale;
       const localDomain = A.localSubdomain();
       const iterPerThread = divceil(localDomain.size, maxThreads);
       var localResult: eltType = 0;
       var threadResults: [0..#maxThreads] eltType = 0;
 
-      coforall tid in 0..#maxThreads {
+      coforall tid in 0..#maxThreads with (ref threadResults) {
         const startid = localDomain.lowBound + tid * iterPerThread;
         const temp_endid = startid + iterPerThread - 1;
         const endid = if localDomain.highBound < temp_endid

modules/packages/MPI.chpl

@@ -236,7 +236,7 @@ module MPI {
   /* Module level deinit */
   proc deinit() {
     if _freeChplComm {
-      coforall loc in Locales do on loc {
+      coforall loc in Locales with (ref CHPL_COMM_WORLD_REPLICATED) do on loc {
           C_MPI.MPI_Comm_free(CHPL_COMM_WORLD_REPLICATED(1));
         }
     }
@@ -319,7 +319,7 @@ module MPI {
   @chpldoc.nodoc
   proc setChplComm() {
     if numLocales > 1 {
-      coforall loc in Locales do on loc {
+      coforall loc in Locales with (ref CHPL_COMM_WORLD_REPLICATED) do on loc {
         C_MPI.MPI_Comm_split(MPI_COMM_WORLD, 0, here.id : c_int,
           CHPL_COMM_WORLD_REPLICATED(1));
       }

modules/packages/Sort.chpl

@@ -2853,7 +2853,7 @@ module TwoArrayPartitioning {
       //       state, criterion, task.startbit);
 
       coforall (bktLoc, bktLocId, bktTask) in distTask.localesAndTasks(A)
-      with (ref state1, ref state2, ref nextDistTaskElts) do
+      with (ref state1, ref state2, ref nextDistTaskElts, ref smallTasksPerLocale) do
       on bktLoc {
         // Each bucket can run in parallel - this allows each
         // bucket to use nested coforalls to barrier.
@@ -3127,7 +3127,7 @@ module TwoArrayPartitioning {
 
     // Always use state 1 for small subproblems...
     ref state = state1;
-    coforall (loc,tid) in zip(A.targetLocales(),0..) with (ref state) do
+    coforall (loc,tid) in zip(A.targetLocales(),0..) with (ref state, ref smallTasksPerLocale) do
     on loc {
       // Get the tasks to sort here
 

modules/standard/DynamicIters.chpl

@@ -521,7 +521,7 @@ where tag == iterKind.leader
     var barrier : atomic int;
 
     // Start the parallel work
-    coforall tid in 0..#nTasks with (const in r) {
+    coforall tid in 0..#nTasks with (const in r, ref localWork, ref moreLocalWork, ref locks) {
 
       // Step 1: Initial range per Thread/Task
 

test/arrays/diten/replaceArrayAccess/modifyInAnotherTask.chpl

@@ -4,7 +4,7 @@ var s: sync bool;
 proc f(i) do
   return i;
 
-begin {
+begin with (ref A) {
   for i in 1..4 {
     A[i] = i;
     s.writeEF(true);

test/arrays/diten/replaceArrayAccess/modifyInAnotherTask2.chpl

@@ -4,7 +4,7 @@ var s: sync bool;
 proc f(i) do
   return i;
 
-begin {
+begin with (ref A) {
   for i in 1..4 {
     A[i] = i;
     s = true;

test/arrays/ferguson/semantic-examples/4-tasks-on-check.chpl

@@ -3,7 +3,7 @@ proc test_begin() {
 
   var A:[1..10] int;
   sync {
-    begin {
+    begin with (ref A) {
       A[1] = 1;
     }
   }
@@ -16,7 +16,7 @@ proc test_begin_on() {
 
   var A:[1..10] int;
   sync {
-    begin on Locales[numLocales-1] {
+    begin with (ref A) on Locales[numLocales-1] {
       A[1] = 1;
     }
   }
@@ -27,7 +27,7 @@ proc test_begin_on() {
 proc test_cobegin() {
 
   var A:[1..10] int;
-  cobegin {
+  cobegin with (ref A) {
     A[1] = 1;
     A[2] = 2;
   }
@@ -38,7 +38,7 @@ proc test_cobegin() {
 proc test_cobegin_on() {
 
   var A:[1..10] int;
-  cobegin {
+  cobegin with (ref A) {
     on Locales[numLocales-1] do A[1] = 1;
     on Locales[numLocales-1] do A[2] = 2;
   }
@@ -50,7 +50,7 @@ proc test_cobegin_on() {
 proc test_coforall() {
 
   var A:[1..10] int;
-  coforall i in 1..10 {
+  coforall i in 1..10 with (ref A) {
     A[i] = i;
   }
 
@@ -60,7 +60,7 @@ proc test_coforall() {
 proc test_coforall_on() {
 
   var A:[1..10] int;
-  coforall i in 1..10 {
+  coforall i in 1..10 with (ref A) {
     on Locales[numLocales-1] do A[i] = i;
   }