Convert Private class to privateDist record (chapel-lang#23393)

[reviewed by @benharsh]

Following in the steps of other recent distribution-related PRs, this
converts the `Private` class to a `privateDist` record as a step towards
deprecating support for the `dmap` type. Where previous PRs have done
this as two separate steps, this one does it all at once.

This one is a bit less satisfying than previous ones, primarily because
`PrivateDist` is less satisfying than other distributions. For example,
it leaks memory when anything other than `PrivateSpace` is used, and
existing testing doesn't really use anything other than `PrivateSpace`.
As a result, I incorporated all the same boilerplate and patterns as for
the other distributions that have been converted, and made
`PrivateSpace` better as a result (e.g., you can use copy initialization
with it now!), yet there aren't many tests that show it's working better
or worse than before.

My deprecated dmapType.chpl test was intended to be such a test, and it
does work, but it also demonstrates the aforementioned memory leak, so I
put that code into a conditional that doesn't run to avoid having to
deal with that while still checking that the deprecation warnings are
working.

While here, I realized for the first time that my dmapType.chpl test
didn't run using multiple locales even though it's exercising
distributions and could, so I added a numlocales file to lock that in.

modules/dists/PrivateDist.chpl

@@ -22,16 +22,17 @@
 prototype module PrivateDist {
 
 use ChplConfig only compiledForSingleLocale;
+use DSIUtil;
 
 //
 // Private Distribution, Domain, and Array
 //  Defines PrivateSpace, an instance of PrivateDom
 //
 /*
-This Private distribution maps each index ``i``
+This ``privateDist`` distribution maps each index ``i``
 between ``0`` and ``numLocales-1`` to ``Locales[i]``.
 
-The index set of a domain distributed over a Private distribution
+The index set of a domain distributed over ``privateDist``
 is always ``0..numLocales-1``, regardless of the domain's rank,
 and cannot be changed.
 
@@ -40,7 +41,7 @@ so user programs do not need to declare their own:
 
   .. code-block:: chapel
 
-    const PrivateSpace: domain(1) dmapped Private();
+    const PrivateSpace: domain(1) dmapped privateDist();
 
 
 **Example**
@@ -58,7 +59,7 @@ corresponding to that locale to that locale's number of cores.
 
 **Data-Parallel Iteration**
 
-A `forall` loop over a Private-distributed domain or array
+A `forall` loop over a ``privateDist`` domain or array
 runs a single task on each locale.
 That task executes the loop's iteration corresponding to
 that locale's index in the ``Locales`` array.
@@ -72,7 +73,89 @@ do not provide some standard domain/array functionality.
 This distribution may perform unnecessary communication
 between locales.
 */
-class Private: BaseDist, writeSerializable {
+
+record privateDist: writeSerializable {
+  forwarding const chpl_distHelp: chpl_PrivatizedDistHelper(unmanaged PrivateImpl);
+
+  proc init() {
+    const value = new unmanaged PrivateImpl();
+    this.chpl_distHelp = new chpl_PrivatizedDistHelper(
+                          if _isPrivatized(value)
+                            then _newPrivatizedClass(value)
+                            else nullPid,
+                          value);
+  }
+
+    proc init(_pid : int, _instance, _unowned : bool) {
+      this.chpl_distHelp = new chpl_PrivatizedDistHelper(_pid,
+                                                         _instance,
+                                                         _unowned);
+    }
+
+    proc init(value) {
+      this.chpl_distHelp = new chpl_PrivatizedDistHelper(
+                             if _isPrivatized(value)
+                               then _newPrivatizedClass(value)
+                               else nullPid,
+                             _to_unmanaged(value));
+    }
+
+    // does not match the type of 'other'. That case is handled by the compiler
+    // via coercions.
+    proc init=(const ref other : privateDist) {
+      this.init(other._value.dsiClone());
+    }
+
+    proc clone() {
+      return new privateDist(this._value.dsiClone());
+    }
+
+  @chpldoc.nodoc
+  inline operator ==(d1: privateDist, d2: privateDist) {
+    if (d1._value == d2._value) then
+      return true;
+    return d1._value.dsiEqualDMaps(d2._value);
+  }
+
+  @chpldoc.nodoc
+  inline operator !=(d1: privateDist, d2: privateDist) {
+    return !(d1 == d2);
+  }
+
+  proc writeThis(x) {
+    chpl_distHelp.writeThis(x);
+  }
+
+  proc serialize(writer, ref serializer) throws {
+    writeThis(writer);
+  }
+}
+
+
+@chpldoc.nodoc
+@unstable(category="experimental", reason="assignment between distributions is currently unstable due to lack of testing")
+operator =(ref a: privateDist, b: privateDist) {
+  if a._value == nil {
+    __primitive("move", a, chpl__autoCopy(b.clone(), definedConst=false));
+  } else {
+    if a._value.type != b._value.type then
+      compilerError("type mismatch in distribution assignment");
+    if a._value == b._value {
+      // do nothing
+    } else
+        a._value.dsiAssign(b._value);
+    if _isPrivatized(a._instance) then
+      _reprivatize(a._value);
+  }
+}
+
+
+@deprecated("'Private' is deprecated, please use 'privateDist' instead")
+type Private = privateDist;
+
+
+@chpldoc.nodoc
+class PrivateImpl: BaseDist, writeSerializable {
   override proc dsiNewRectangularDom(param rank: int, type idxType,
                                      param strides: strideKind, inds) {
     for i in inds do
@@ -103,7 +186,7 @@ class Private: BaseDist, writeSerializable {
 }
 
 class PrivateDom: BaseRectangularDom(?) {
-  var dist: unmanaged Private;
+  var dist: unmanaged PrivateImpl;
 
   iter these() { for i in 0..numLocales-1 do yield i; }
 
@@ -171,6 +254,13 @@ class PrivateDom: BaseRectangularDom(?) {
   proc dsiMember(i) do return (0 <= i && i <= numLocales-1);
 
   override proc dsiMyDist() do return dist;
+
+  proc dsiGetDist() {
+    if _isPrivatized(dist) then
+      return new privateDist(dist.pid, dist, _unowned=true);
+    else
+      return new privateDist(nullPid, dist, _unowned=true);
+}
 }
 
 private proc checkCanMakeDefaultValue(type eltType) param {
@@ -353,11 +443,11 @@ proc PrivateArr.doiScan(op, dom) where (rank == 1) &&
 // deinitializer, which is called before deinitializing module-scope variables
 // which would cause use-after-free during the cleanup of PrivateSpace
 var chpl_privateCW = new chpl_privateDistCleanupWrapper();
-var chpl_privateDist = new unmanaged Private();
-const PrivateSpace: domain(1) dmapped new dmap(chpl_privateDist);
+var chpl_privateDist = new unmanaged PrivateImpl();
+const PrivateSpace: domain(1) dmapped new privateDist(chpl_privateDist);
 
 record chpl_privateDistCleanupWrapper {
-  var val = nil : unmanaged Private?;
+  var val = nil : unmanaged PrivateImpl?;
   proc deinit() { delete val!; }
 }
 

test/deprecated/dmapType.chpl

@@ -1,4 +1,4 @@
-config var n = 10;
+config var n = 10, addLeaks=false;
 {
   use BlockDist;
   var Dist = new dmap(new Block({1..n}));
@@ -38,3 +38,27 @@ config var n = 10;
   const Dist2: dmap(Stencil(1)) = new Stencil({1..n});
   writeln(Dist2.type:string);
 }
+
+{
+  // Private distributions leak, and it's not clear they're
+  // ready for prime-time which is why this test of the
+  // deprecation warning puts most of the code into a
+  // conditional that doesn't run.  This checks that the
+  // deprecation warnings still fire without dealing with
+  // the leaks.  The small test outside of the conditional
+  // is leak-free and shows that the type exists even if
+  // it doesn't do anything interesting with it.
+
+  use PrivateDist;
+  proc foo(type t) { writeln("foo got ", t: string); }
+  foo(Private);
+
+  if addLeaks {
+    var Dist = new dmap(new Private());
+    var Dom: domain(1) dmapped Dist;
+    var A: [Dom] real;
+    writeln(A);
+    const Dist2: dmap(Private) = new Private();
+    writeln(Dist2.type:string);
+  }
+}

test/deprecated/dmapType.good

@@ -10,6 +10,10 @@ dmapType.chpl:28: warning: 'Replicated' is deprecated, please use 'replicatedDis
 dmapType.chpl:34: warning: 'Stencil' is deprecated, please use 'stencilDist' instead
 dmapType.chpl:38: warning: 'Stencil' is deprecated, please use 'stencilDist' instead
 dmapType.chpl:38: warning: 'Stencil' is deprecated, please use 'stencilDist' instead
+dmapType.chpl:54: warning: 'Private' is deprecated, please use 'privateDist' instead
+dmapType.chpl:57: warning: 'Private' is deprecated, please use 'privateDist' instead
+dmapType.chpl:61: warning: 'Private' is deprecated, please use 'privateDist' instead
+dmapType.chpl:61: warning: 'Private' is deprecated, please use 'privateDist' instead
 dmapType.chpl:4: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new <DistName>(<args>))' with 'new <DistName>(<args>)'
 dmapType.chpl:8: warning: The use of 'dmap' is deprecated for this distribution; please replace 'dmap(<DistName>(<args>))' with '<DistName>(<args>)'
 dmapType.chpl:14: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new <DistName>(<args>))' with 'new <DistName>(<args>)'
@@ -18,6 +22,8 @@ dmapType.chpl:24: warning: The use of 'dmap' is deprecated for this distribution
 dmapType.chpl:28: warning: The use of 'dmap' is deprecated for this distribution; please replace 'dmap(<DistName>(<args>))' with '<DistName>(<args>)'
 dmapType.chpl:34: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new <DistName>(<args>))' with 'new <DistName>(<args>)'
 dmapType.chpl:38: warning: The use of 'dmap' is deprecated for this distribution; please replace 'dmap(<DistName>(<args>))' with '<DistName>(<args>)'
+dmapType.chpl:57: warning: The use of 'dmap' is deprecated for this distribution; please replace 'new dmap(new <DistName>(<args>))' with 'new <DistName>(<args>)'
+dmapType.chpl:61: warning: The use of 'dmap' is deprecated for this distribution; please replace 'dmap(<DistName>(<args>))' with '<DistName>(<args>)'
 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0
 blockDist(1,int(64),unmanaged DefaultDist)
 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0
@@ -26,3 +32,4 @@ cyclicDist(1,int(64))
 replicatedDist
 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0 0.0
 stencilDist(1,int(64),false)
+foo got privateDist

test/deprecated/dmapType.numlocales

@@ -0,0 +1 @@
+4