Add/improve errors for .localAccess() calls to non-local elements

modules/dists/BlockDist.chpl

@@ -1917,6 +1917,12 @@ proc BlockImpl.chpl__locToLocIdx(loc: locale) {
   return (false, targetLocDom.first);
 }
 
+iter BlockImpl.chpl__locToLocIdxs(loc: locale) {
+  for locIdx in targetLocDom do
+    if (targetLocales[locIdx] == loc) then
+      yield locIdx;
+}
+
 // Block subdomains are continuous
 
 proc BlockArr.dsiHasSingleLocalSubdomain() param do return !allowDuplicateTargetLocales;
@@ -1953,6 +1959,17 @@ proc BlockDom.dsiLocalSubdomain(loc: locale) {
     return d;
   }
 }
+iter BlockDom.dsiLocalSubdomains(loc: locale) {
+  for locid in dist.chpl__locToLocIdxs(loc) {
+    var inds = chpl__computeBlock(locid, dist.targetLocDom, dist.boundingBox, dist.boundingBox.dims());
+    yield whole[(...inds)];
+  }
+}
+iter BlockArr.dsiLocalSubdomains(loc: locale) {
+  for subdoms in dom.dsiLocalSubdomains(loc) {
+    yield subdoms;
+  }
+}
 
 proc BlockDom.numRemoteElems(viewDom, rlo, rid) {
   // NOTE: Not bothering to check to see if rid+1, length, or rlo-1 used

modules/dists/CyclicDist.chpl

@@ -483,6 +483,12 @@ proc CyclicImpl.chpl__locToLocIdx(loc: locale) {
   return (false, targetLocDom.first);
 }
 
+iter CyclicImpl.chpl__locToLocIdxs(loc: locale) {
+  for locIdx in targetLocDom do
+    if (targetLocs[locIdx] == loc) then
+      yield locIdx;
+}
+
 proc CyclicImpl.getChunk(inds, locid) {
   const chunk = locDist(locid).myChunk((...inds.getIndices()));
   return chunk;
@@ -1621,6 +1627,17 @@ proc CyclicDom.dsiLocalSubdomain(loc: locale) {
     return d;
   }
 }
+iter CyclicDom.dsiLocalSubdomains(loc: locale) {
+  for locid in dist.chpl__locToLocIdxs(loc) {
+    var inds = chpl__computeCyclic(idxType, locid, dist.targetLocDom.dims(), dist.startIdx);
+    yield whole[(...inds)];
+  }
+}
+iter CyclicArr.dsiLocalSubdomains(loc: locale) {
+  for subdoms in dom.dsiLocalSubdomains(loc) {
+    yield subdoms;
+  }
+}
 
 proc CyclicArr.canDoOptimizedSwap(other) {
   var domsMatch = true;

modules/dists/StencilDist.chpl

@@ -2313,6 +2313,12 @@ proc StencilImpl.chpl__locToLocIdx(loc: locale) {
   return (false, targetLocDom.first);
 }
 
+iter StencilImpl.chpl__locToLocIdxs(loc: locale) {
+  for locIdx in targetLocDom do
+    if (targetLocales[locIdx] == loc) then
+      yield locIdx;
+}
+
 // Stencil subdomains are continuous
 
 proc StencilArr.dsiHasSingleLocalSubdomain() param do return !allowDuplicateTargetLocales;
@@ -2346,6 +2352,37 @@ proc StencilDom.dsiLocalSubdomain(loc: locale) {
     return d;
   }
 }
+iter StencilDom.dsiLocalSubdomains(loc: locale) {
+  for locid in dist.chpl__locToLocIdxs(loc) {
+    var inds = chpl__computeBlock(locid, dist.targetLocDom, dist.boundingBox, dist.boundingBox.dims());
+    yield whole[(...inds)];
+  }
+}
+iter StencilArr.dsiLocalSubdomains(loc: locale) {
+  for subdoms in dom.dsiLocalSubdomains(loc) {
+    yield subdoms;
+  }
+}
+
+// These are similar to the normal dsiLocalSubdomains() itertors, but
+// they yield the local blocks as extended by the fluff for the sake
+// of locality-checking rather than just the local blocks.
+//
+iter StencilDom.doiLocalStoredSubdomains() {
+  for locid in dist.chpl__locToLocIdxs(here) {
+
+    // Could calculate the indices directly, as in the normal
+    // subdomains case above, but that's more complicated for
+    // StencilDist, and this isn't on performance-critical paths
+    // anyway, so I'm just looking it up.
+    yield locDoms[locid].myFluff;
+  }
+}
+iter StencilArr.doiLocalStoredSubdomains() {
+  for subdom in dom.doiLocalStoredSubdomains() do {
+    yield subdom;
+  }
+}
 
 proc StencilDom.numRemoteElems(viewDom, rlo, rid) {
   // NOTE: Not bothering to check to see if rid+1, length, or rlo-1 used

modules/internal/ChapelArray.chpl

@@ -848,7 +848,7 @@ module ChapelArray {
     pragma "insert line file info"
     pragma "always propagate line file info"
     @chpldoc.nodoc
-    proc checkAccess(indices, value) {
+    proc checkAccess(indices, value, param ensureLocal=false) {
       if this.isRectangular() {
         if !value.dsiBoundsCheck(indices) {
           if rank == 1 {
@@ -882,6 +882,29 @@ module ChapelArray {
                  "note: ", dimstr);
           }
         }
+        if ensureLocal {
+          const locInds = this.chpl__localStoredSubdomains(),
+                locIdx = || reduce for blk in locInds do blk.contains(indices);
+
+          if !locIdx {
+            if chpl_isNonDistributedArray() {
+              halt("Cannot use .localAccess() from locale ", here.id,
+                   " on a remote array stored on locale ", value.locale.id);
+            } else {
+              if locInds.size == 1 && locInds[0].size == 0 {
+                halt("Call to .localAccess() from locale ", here.id,
+                     " is illegal because it has no local array elements");
+              } else {
+                halt("Call to .localAccess",
+                     if indices.size == 1 then "(" + indices(0):string + ")"
+                                          else indices,
+                     " from locale ", here.id,
+                     " refers to remote data (local indices are: ", locInds,
+                     ")");
+              }
+            }
+          }
+        }
       }
     }
 
@@ -1031,7 +1054,7 @@ module ChapelArray {
     {
       const value = _value;
       if boundsChecking then
-        checkAccess(i, value=value);
+        checkAccess(i, value=value, ensureLocal=true);
 
       if logAllArrEltAccess ||
         (logDistArrEltAccess && !chpl_isNonDistributedArray()) then
@@ -1052,7 +1075,7 @@ module ChapelArray {
     {
       const value = _value;
       if boundsChecking then
-        checkAccess(i, value=value);
+        checkAccess(i, value=value, ensureLocal=true);
 
       if logAllArrEltAccess ||
         (logDistArrEltAccess && !chpl_isNonDistributedArray()) then
@@ -1072,7 +1095,7 @@ module ChapelArray {
     {
       const value = _value;
       if boundsChecking then
-        checkAccess(i, value=value);
+        checkAccess(i, value=value, ensureLocal=true);
 
       if logAllArrEltAccess ||
         (logDistArrEltAccess && !chpl_isNonDistributedArray()) then
@@ -1615,6 +1638,24 @@ module ChapelArray {
       }
     }
 
+    // This internal iterator is like 'localSubdomains()' but gives a
+    // distribution the option of indicating that it stores more
+    // indices than just those in its local subdomain; The stencil
+    // distribution is such an example since each locale stores cached
+    // boundary values ("fluff") in addition to its true local
+    // subdomain.  Thus, locality checks should pass if they're within
+    // the fluff even though that's outside of the normal
+    // localSubdomain() index set.
+    @chpldoc.nodoc
+    iter chpl__localStoredSubdomains() {
+      use Reflection;
+      if canResolveMethod(_value, "doiLocalStoredSubdomains") {
+        for d in _value.doiLocalStoredSubdomains() do yield d;
+      } else {
+        for d in localSubdomains() do yield d;
+      }
+    }
+
     proc chpl__isDense1DArray() param {
       return this.isRectangular() &&
              this.rank == 1 &&

test/arrays/locality/localAccess.skipif

@@ -0,0 +1,2 @@
+CHPL_COMM==none
+COMPOPTS <= --no-local

test/arrays/locality/localAccess/COMPOPTS

@@ -0,0 +1,2 @@
+-sPOD=true
+-sPOD=false

test/arrays/locality/localAccess/NUMLOCALES

@@ -0,0 +1 @@
+2

test/arrays/locality/localAccess/localAccess-block.chpl

@@ -0,0 +1,29 @@
+use BlockDist;
+config param POD = true;
+config const read = true;
+
+var Arr = createArray();
+
+on Locales[numLocales-1] {
+  if read then writeln(Arr.localAccess[0]); else Arr.localAccess[0] += 1;
+}
+
+proc createArray() {
+  const D = {0..2} dmapped new blockDist({0..99});
+  var A: [D] if POD then int else R;
+  if POD then
+    A = [1, 2, 3];
+  else
+    A = [new R(1), new R(2), new R(3)];
+  return A;
+}
+
+record R {
+  var x: int;
+
+  operator +(ref lhs: R, rhs: int) {
+    lhs.x += rhs;
+    return lhs;
+  }
+}
+

test/arrays/locality/localAccess/localAccess-block.execopts

@@ -0,0 +1,2 @@
+--read=true
+--read=false

test/arrays/locality/localAccess/localAccess-block.good

@@ -0,0 +1 @@
+localAccess-block.chpl:8: error: halt reached - Call to .localAccess() from locale 1 is illegal because it has no local array elements

test/arrays/locality/localAccess/localAccess-block2-2D.chpl

@@ -0,0 +1,29 @@
+use BlockDist;
+config param POD = true;
+config const read = true;
+
+var Arr = createArray();
+
+on Locales[numLocales-1] {
+  if read then writeln(Arr.localAccess[0,0]); else Arr.localAccess[0,0] += 1;
+}
+
+proc createArray() {
+  const D = {0..99 by 50, 0..99 by 50} dmapped new blockDist({0..99, 0..99});
+  var A: [D] if POD then int else R;
+  if POD then
+    A = reshape([1, 2, 3, 4], A.domain);
+  else
+    A = reshape([new R(1), new R(2), new R(3), new R(4)], A.domain);
+  return A;
+}
+
+record R {
+  var x: int;
+
+  operator +(ref lhs: R, rhs: int) {
+    lhs.x += rhs;
+    return lhs;
+  }
+}
+

test/arrays/locality/localAccess/localAccess-block2-2D.execopts

@@ -0,0 +1,2 @@
+--read=true
+--read=false

test/arrays/locality/localAccess/localAccess-block2-2D.good

@@ -0,0 +1 @@
+localAccess-block2-2D.chpl:8: error: halt reached - Call to .localAccess(0, 0) from locale 1 refers to remote data (local indices are: {50..99 by 50, 0..99 by 50})

test/arrays/locality/localAccess/localAccess-block2.chpl

@@ -0,0 +1,29 @@
+use BlockDist;
+config param POD = true;
+config const read = true;
+
+var Arr = createArray();
+
+on Locales[numLocales-1] {
+  if read then writeln(Arr.localAccess[0]); else Arr.localAccess[0] += 1;
+}
+
+proc createArray() {
+  const D = {0..99 by 34} dmapped new blockDist({0..99});
+  var A: [D] if POD then int else R;
+  if POD then
+    A = [1, 2, 3];
+  else
+    A = [new R(1), new R(2), new R(3)];
+  return A;
+}
+
+record R {
+  var x: int;
+
+  operator +(ref lhs: R, rhs: int) {
+    lhs.x += rhs;
+    return lhs;
+  }
+}
+

test/arrays/locality/localAccess/localAccess-block2.execopts

@@ -0,0 +1,2 @@
+--read=true
+--read=false

test/arrays/locality/localAccess/localAccess-block2.good

@@ -0,0 +1 @@
+localAccess-block2.chpl:8: error: halt reached - Call to .localAccess(0) from locale 1 refers to remote data (local indices are: {50..99 by 34 align 0})

test/arrays/locality/localAccess/localAccess.chpl

@@ -0,0 +1,26 @@
+config param POD = true;
+
+config const read = true;
+
+var Arr = createArray();
+
+on Locales[numLocales-1] {
+  if read then writeln(Arr.localAccess[0]); else Arr.localAccess[0] += 1;
+}
+
+proc createArray() {
+  if POD then
+    return [1, 2, 3];
+  else
+    return [new R(1), new R(2), new R(3)];
+}
+
+record R {
+  var x: int;
+
+  operator +(ref lhs: R, rhs: int) {
+    lhs.x += rhs;
+    return lhs;
+  }
+}
+

test/arrays/locality/localAccess/localAccess.execopts

@@ -0,0 +1,2 @@
+--read=true
+--read=false

test/arrays/locality/localAccess/localAccess.good

@@ -0,0 +1 @@
+localAccess.chpl:8: error: halt reached - Cannot use .localAccess() from locale 1 on a remote array stored on locale 0

test/distributions/block/duplicateTargetLocales.allow.good

@@ -1,3 +1,3 @@
 1 0 0 0 0 0 0 0 0 10
-1 0 0 0 0 0 0 0 0 10
+1 0 0 0 0 0 7 0 0 0
 1 3 6 10 15 21 28 36 45 55

test/distributions/block/duplicateTargetLocales.chpl

@@ -16,7 +16,7 @@ writeln(A);
 
 A = 0;
 A.localAccess[1] = 1;
-A.localAccess[n] = n;
+A.localAccess[7] = 7;
 writeln(A);
 
 A = space;

test/distributions/cyclic/duplicateTargetLocales.allow.good

@@ -1,4 +1,4 @@
 duplicateTargetLocales.chpl:23: warning: scan has been serialized (see issue #12482)
 1 0 0 0 0 0 0 0 0 10
-1 0 0 0 0 0 0 0 0 10
+1 0 0 0 0 0 0 0 9 0
 1 3 6 10 15 21 28 36 45 55

test/distributions/cyclic/duplicateTargetLocales.chpl

@@ -16,7 +16,7 @@ writeln(A);
 
 A = 0;
 A.localAccess[1] = 1;
-A.localAccess[n] = n;
+A.localAccess[9] = 9;
 writeln(A);
 
 A = space;

test/distributions/stencil/localAccess.chpl

@@ -0,0 +1,21 @@
+use StencilDist;
+
+const D = {1..10} dmapped new stencilDist({1..10}, fluff=(1,));
+var A: [D] real = [i in D] i;
+
+A.updateFluff();
+
+// on a 2-locale run, these accesses would be considered out-of-bounds
+// with respect to the indices that the locale is the primary owner
+// of, but not with respect to the fluff values that it stores.  This
+// test ensures that we don't get an OOB access check on such local
+// accesses to fluff values.
+
+writeln(A.localAccess(5));    // this is properly owned by me
+writeln(A.localAccess(6));    // this is part of my fluff
+
+on Locales.last {
+  writeln(A.localAccess(6));  // this is properly owned by me
+  writeln(A.localAccess(5));  // this is part of my fluff
+  writeln(A.localAccess(4));  // this is not local
+}