From 80f6d116b2d3bb8c7aa7466ae6c0937ef1f8e28a Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Wed, 15 May 2019 11:17:59 -0700
Subject: [PATCH 01/11] Work in progress draft of new List API

---
 modules/standard/Lists.chpl | 302 ++++++++++++++++++++++++++++++++++++
 1 file changed, 302 insertions(+)
 create mode 100644 modules/standard/Lists.chpl

diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
new file mode 100644
index 000000000000..99d7f9dc63de
--- /dev/null
+++ b/modules/standard/Lists.chpl
@@ -0,0 +1,302 @@
+/*
+ * Copyright 2004-2019 Cray Inc.
+ * Other additional copyright holders may be indicated within.
+ *
+ * The entirety of this work is licensed under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License.
+ *
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+  (Word smithing module text below).
+
+  ---
+
+  This module contains the implementation of a list datatype, intended to
+  replace the use of arrays to perform "vector-like" operations.
+
+  A list is a lightweight container intended to mimic Python's list datatype,
+  with fast O(log n)/O(1) (whatever we end up on) indexing  and an API close
+  in spirit to its Python counterpart.
+
+  The highly parallel nature of Chapel means that great care should be taken
+  when performing operations which may trigger a move of list elements.
+  Inserts and removals into the middle of a list are example operations which
+  may trigger a move, thus invalidating all references to elements of the
+  list held by tasks before the move. Appending an element to the end of a
+  list will never cause a move.
+
+  The following operations may trigger a move:
+    - insert
+    - remove
+    - reverse
+    - sort
+    - pop
+
+  ---
+
+  (End word smithing module text).
+
+  I know that @mppf in particular is very interested in limiting the number of
+  operations that move/displace list elements.
+
+  This pertains to the general issue of invalidating references to (and also
+  iterators over) list elements.
+
+  There is also the separate, more general issue of parallel safety. I'd
+  happily appreciate any/all recommendations for the "Chapeltastic" way of
+  making this List type parallel safe.
+
+
+*/
+module ProtoLists {
+
+  // Building blocks used as backing store for lists.
+  pragma "no doc"
+  class ListBlock {
+    type _etype;
+    var data: _ddata(_etype) = nil;
+  }
+
+  /*
+    A container intended to replace the use of arrays to perform "vector-like"
+    operations.
+
+    Is not currently parallel safe.
+
+    The following operations may trigger a move:
+      - insert
+      - remove
+      - reverse
+      - sort
+      - pop
+
+    A move will invalidate any references to list elements held by tasks, as
+    well as iterators to list elements produced by tasks before the move
+    occurred.
+  */
+  class List {
+
+    pragma "no doc"
+    var _size = 0;
+    pragma "no doc"
+    var _etype;
+
+    //
+    // Michael suggested that we move from a LL of blocks to an array, which
+    // would give us constant speed indexing at the cost of more complicated
+    // indexing logic.
+    //
+    pragma "no doc"
+    var _head, _tail: unmanaged ListBlock(_etype) = nil;
+
+
+    /*
+      Initializes a new List containing elements of the given type.
+
+      :arg etype: The type of the elements of this List.
+    */
+    proc init(type etype) { _etype = etype; }
+
+
+    /*
+      Initializes a new List as a copy of another List object.
+
+      :arg other: The List object to initialize from.
+    */
+    proc init=(other: List) {
+      this._etype = other._etype;
+    }
+  
+
+    proc deinit() {}
+
+
+    /*
+      Add an element to the end of this list.
+
+      .. note::
+
+        Appending an element to a list will never invalidate references to
+        (or iterators over) the elements of the list.
+
+      :arg x: An element to append.
+    */
+    proc append(x: this._etype) {}
+
+
+    /*
+      Extend this List by appending all the elements from another List.
+
+      :arg other: A List, the elements of which are appended to this List.
+    */
+    proc extend(other: List(this._etype)) {}
+
+
+    /*
+      Extend this List by appending all the elements from an array.
+
+      :arg other: An array, the elements of which are appended to this List.
+    */
+    proc extend(other: this._etype[?d]) {}
+
+
+    /*
+      Insert an element at a given position in this List, shifting all elements
+      following that index one to the right. Note that `a.insert(a.size, x)`
+      is equivalent to `a.append(x)`.
+
+      :arg i: The index of the element at which to insert.
+      :arg x: The element to insert.
+
+      :throws IllegalArgumentError: If the given index is out of bounds.
+    */
+    proc insert(i: int, x: this._etype) throws {}
+
+
+    /*
+      Remove the first item from the list whose value is equal to _x_.
+
+      :arg x: The element to remove.
+
+      :throws IllegalArgumentError: If there is no such item.
+    */
+    proc remove(x: this._etype) throws {}
+
+
+    /*
+      Remove the item at the given position in this List, and return it. If no
+      index is specified, remove and return the last item in this List.
+
+      :arg i: The index of the element to remove. Defaults to the last item
+        in this list.
+      
+      :return: The item removed.
+
+      :throws IllegalArgumentError: If the given index is out of bounds.
+    */
+    proc pop(i: int=this._size): this._etype throws { return nil; }
+
+
+    /*
+      Clear the contents of this List.
+    */
+    proc clear() {}
+
+    /*
+      Return a zero-based index in the list of the first item whose value is
+      equal to _x_.
+
+      :arg x: An element to search for.
+
+      :return: The index of the element to search for.
+
+      :throws IllegalArgumentError: If the given element cannot be found.
+    */
+    proc indexOf(x: _etype, start: int=0, end: int=_size): int throws {
+      return 0; 
+    }
+
+    /*
+      Return the number of times _x_ appears in the list.
+
+      :arg x: An element to count.
+
+      :return: The number of times a given element is found in this List.
+      :rtype: `int`
+    */
+    proc count(x: this._etype): int {
+      return 0;
+    }
+
+    /*
+      Sort the items of this List in place. The parameters of this method are
+      holdovers from Python. This method could more closely integrate with
+      Chapel sorting APIs.
+
+      :arg key: Unused in current API.
+      :arg reverse: True if this List should be sorted in reverse order.
+
+    */
+    proc sort(key: this._etype=nil, reverse: bool=false) {}
+
+
+    /*
+      Reverse the elements of this list in place.
+    */
+    proc reverse() {}
+
+    /*
+      Index this list via subscript. Returns a reference to the element at a
+      given index in this List.
+
+      :arg i: The index of the element to access.
+
+      :return: An element from this List.
+
+      :throws IllegalArgumentError: If the given index is out of bounds.
+    */
+    proc this(i: int) ref throws { return nil; }
+
+
+    /*
+      Produce a serial iterator over the elements of this List.
+    */
+    iter these() ref { yield _size; }
+
+
+    /*
+      Write the contents of this List to a given channel.
+
+      :arg ch: A channel to write to.
+    */
+    proc writeThis(ch: channel) {}
+
+
+    /*
+      Convert the contents of this List to string form. Could be replaced by
+      an overload of the cast operation?
+
+      :return: The contents of this List, in a form suitable for printing.
+      :rtype: `string`
+    */
+    proc toString(): string {}
+
+
+    /*
+      Return the number of elements in this List.
+
+      :return: The number of elements in this List.
+      :rtype: `int`
+    */
+    proc size { return _size; }
+
+    /*
+      Returns `true` if this list contains zero elements.
+
+      :return: `true` if this list is empty.
+      :rtype: `bool`
+    */
+    proc isEmpty(): bool { return true; }
+
+
+  }
+
+
+  proc =(a: List(?t), b: List(t)) {}
+
+  proc ==(a: List(?t), b: List(t)): bool { return true; }
+
+  proc +(a: List(?t), b: List(t)): List(t) { return nil; }
+
+}

From aed738946ada27238c6464705fbdd352098f8275 Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Wed, 15 May 2019 13:32:26 -0700
Subject: [PATCH 02/11] Tidy up comments, list array ops to deprecate/remove

---
 modules/standard/Lists.chpl | 104 +++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 42 deletions(-)

diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
index 99d7f9dc63de..434c6a303925 100644
--- a/modules/standard/Lists.chpl
+++ b/modules/standard/Lists.chpl
@@ -25,16 +25,16 @@
   This module contains the implementation of a list datatype, intended to
   replace the use of arrays to perform "vector-like" operations.
 
-  A list is a lightweight container intended to mimic Python's list datatype,
+  A List is a lightweight container intended to mimic Python's list datatype,
   with fast O(log n)/O(1) (whatever we end up on) indexing  and an API close
   in spirit to its Python counterpart.
 
   The highly parallel nature of Chapel means that great care should be taken
-  when performing operations which may trigger a move of list elements.
-  Inserts and removals into the middle of a list are example operations which
+  when performing operations which may trigger a move of List elements.
+  Inserts and removals into the middle of a List are example operations which
   may trigger a move, thus invalidating all references to elements of the
-  list held by tasks before the move. Appending an element to the end of a
-  list will never cause a move.
+  List held by tasks before the move. Appending an element to the end of a
+  List will never cause a move.
 
   The following operations may trigger a move:
     - insert
@@ -48,20 +48,41 @@
   (End word smithing module text).
 
   I know that @mppf in particular is very interested in limiting the number of
-  operations that move/displace list elements.
+  operations that move/displace List elements.
 
   This pertains to the general issue of invalidating references to (and also
-  iterators over) list elements.
+  iterators over) List elements.
 
   There is also the separate, more general issue of parallel safety. I'd
   happily appreciate any/all recommendations for the "Chapeltastic" way of
   making this List type parallel safe.
 
+  ---
+
+  I believe it is the intention of this new List datatype to replace all
+  "array-as-vec" operations on arrays. That means the following procedures
+  would no longer be supported (deprecated?, or removed?) on arrays:
+
+    - proc push_back(in val: this.eltType)
+    - proc push_back(vals: [])
+    - proc pop_back()
+    - proc push_front(in val: this.eltType)
+    - proc push_front(vals: [])
+    - proc pop_front()
+    - proc insert(pos: this.idxType, in val: this.eltType)
+    - proc insert(pos: this.idxType, vals: [])
+    - proc remove(pos: this.idxType)
+    - proc remove(pos: this.idxType, count: this.idxType)
+    - proc remove(pos: range(this.idxType, stridable = false))
+    - proc clear()
+
+  All of the methods given above change an array's domain (exactly what we
+  are trying to avoid, and the impetus for the List type).
 
 */
-module ProtoLists {
+module Lists {
 
-  // Building blocks used as backing store for lists.
+  // Building blocks used to store "chunks" of List elements.
   pragma "no doc"
   class ListBlock {
     type _etype;
@@ -81,8 +102,8 @@ module ProtoLists {
       - sort
       - pop
 
-    A move will invalidate any references to list elements held by tasks, as
-    well as iterators to list elements produced by tasks before the move
+    A move will invalidate any references to List elements held by tasks, as
+    well as iterators to List elements produced by tasks before the move
     occurred.
   */
   class List {
@@ -97,17 +118,20 @@ module ProtoLists {
     // would give us constant speed indexing at the cost of more complicated
     // indexing logic.
     //
+    // For now, I can start with Vass's logic and then switch over to a O(1)
+    // indexing scheme when everything else is working.
+    //
     pragma "no doc"
     var _head, _tail: unmanaged ListBlock(_etype) = nil;
 
-
     /*
       Initializes a new List containing elements of the given type.
 
       :arg etype: The type of the elements of this List.
     */
-    proc init(type etype) { _etype = etype; }
-
+    proc init(type etype) {
+      _etype = etype;
+    }
 
     /*
       Initializes a new List as a copy of another List object.
@@ -118,23 +142,20 @@ module ProtoLists {
       this._etype = other._etype;
     }
   
-
     proc deinit() {}
 
-
     /*
-      Add an element to the end of this list.
+      Add an element to the end of this List.
 
       .. note::
 
-        Appending an element to a list will never invalidate references to
-        (or iterators over) the elements of the list.
+        Appending an element to a List will never invalidate references to
+        elements in the List.
 
       :arg x: An element to append.
     */
     proc append(x: this._etype) {}
 
-
     /*
       Extend this List by appending all the elements from another List.
 
@@ -142,7 +163,6 @@ module ProtoLists {
     */
     proc extend(other: List(this._etype)) {}
 
-
     /*
       Extend this List by appending all the elements from an array.
 
@@ -150,7 +170,6 @@ module ProtoLists {
     */
     proc extend(other: this._etype[?d]) {}
 
-
     /*
       Insert an element at a given position in this List, shifting all elements
       following that index one to the right. Note that `a.insert(a.size, x)`
@@ -163,9 +182,8 @@ module ProtoLists {
     */
     proc insert(i: int, x: this._etype) throws {}
 
-
     /*
-      Remove the first item from the list whose value is equal to _x_.
+      Remove the first item from this List whose value is equal to _x_.
 
       :arg x: The element to remove.
 
@@ -173,20 +191,18 @@ module ProtoLists {
     */
     proc remove(x: this._etype) throws {}
 
-
     /*
       Remove the item at the given position in this List, and return it. If no
       index is specified, remove and return the last item in this List.
 
       :arg i: The index of the element to remove. Defaults to the last item
-        in this list.
+        in this List, or `a.remove(a.size - 1)`.
       
       :return: The item removed.
 
       :throws IllegalArgumentError: If the given index is out of bounds.
     */
-    proc pop(i: int=this._size): this._etype throws { return nil; }
-
+    proc pop(i: int=this._size - 1): this._etype throws { return nil; }
 
     /*
       Clear the contents of this List.
@@ -194,8 +210,8 @@ module ProtoLists {
     proc clear() {}
 
     /*
-      Return a zero-based index in the list of the first item whose value is
-      equal to _x_.
+      Return a zero-based index into this List of the first item whose value
+      is equal to _x_.
 
       :arg x: An element to search for.
 
@@ -208,7 +224,7 @@ module ProtoLists {
     }
 
     /*
-      Return the number of times _x_ appears in the list.
+      Return the number of times _x_ appears in this List.
 
       :arg x: An element to count.
 
@@ -230,14 +246,13 @@ module ProtoLists {
     */
     proc sort(key: this._etype=nil, reverse: bool=false) {}
 
-
     /*
-      Reverse the elements of this list in place.
+      Reverse the elements of this List in place.
     */
     proc reverse() {}
 
     /*
-      Index this list via subscript. Returns a reference to the element at a
+      Index this List via subscript. Returns a reference to the element at a
       given index in this List.
 
       :arg i: The index of the element to access.
@@ -248,13 +263,11 @@ module ProtoLists {
     */
     proc this(i: int) ref throws { return nil; }
 
-
     /*
       Produce a serial iterator over the elements of this List.
     */
     iter these() ref { yield _size; }
 
-
     /*
       Write the contents of this List to a given channel.
 
@@ -262,7 +275,6 @@ module ProtoLists {
     */
     proc writeThis(ch: channel) {}
 
-
     /*
       Convert the contents of this List to string form. Could be replaced by
       an overload of the cast operation?
@@ -272,7 +284,6 @@ module ProtoLists {
     */
     proc toString(): string {}
 
-
     /*
       Return the number of elements in this List.
 
@@ -282,21 +293,30 @@ module ProtoLists {
     proc size { return _size; }
 
     /*
-      Returns `true` if this list contains zero elements.
+      Returns `true` if this List contains zero elements.
 
-      :return: `true` if this list is empty.
+      :return: `true` if this List is empty.
       :rtype: `bool`
     */
     proc isEmpty(): bool { return true; }
 
-
   }
 
-
   proc =(a: List(?t), b: List(t)) {}
 
   proc ==(a: List(?t), b: List(t)): bool { return true; }
 
+  /*
+    An important question here: do we want the semantics of addition to mirror
+    Chapel arrays, or Python lists?
+
+    Given Chapel semantics, `+` would be a "vectorlike" operation, performing
+    scalar addition against all elements of this List, returning a new List
+    as a result.
+
+    Given Python semantics, `+` would be a shorthand for concatenating two
+    Lists together, returning a new List as a result.
+  */
   proc +(a: List(?t), b: List(t)): List(t) { return nil; }
 
 }

From 57a8f44f8a5d84796989da85a41577f81f114b3d Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Wed, 15 May 2019 13:57:17 -0700
Subject: [PATCH 03/11] Add missing comments and toArray method

---
 modules/standard/Lists.chpl | 58 ++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
index 434c6a303925..ce2971c59278 100644
--- a/modules/standard/Lists.chpl
+++ b/modules/standard/Lists.chpl
@@ -202,7 +202,9 @@ module Lists {
 
       :throws IllegalArgumentError: If the given index is out of bounds.
     */
-    proc pop(i: int=this._size - 1): this._etype throws { return nil; }
+    proc pop(i: int=this._size - 1): this._etype throws {
+      return nil;
+    }
 
     /*
       Clear the contents of this List.
@@ -261,12 +263,19 @@ module Lists {
 
       :throws IllegalArgumentError: If the given index is out of bounds.
     */
-    proc this(i: int) ref throws { return nil; }
+    proc this(i: int) ref throws {
+      return nil;
+    }
 
     /*
       Produce a serial iterator over the elements of this List.
+
+      :yields: A reference to one of the elements contained in this List.
+      :y
     */
-    iter these() ref { yield _size; }
+    iter these() ref {
+      yield _size;
+    }
 
     /*
       Write the contents of this List to a given channel.
@@ -282,7 +291,9 @@ module Lists {
       :return: The contents of this List, in a form suitable for printing.
       :rtype: `string`
     */
-    proc toString(): string {}
+    proc toString(): string {
+      return "";
+    }
 
     /*
       Return the number of elements in this List.
@@ -290,7 +301,9 @@ module Lists {
       :return: The number of elements in this List.
       :rtype: `int`
     */
-    proc size { return _size; }
+    proc size {
+      return _size;
+    }
 
     /*
       Returns `true` if this List contains zero elements.
@@ -298,13 +311,35 @@ module Lists {
       :return: `true` if this List is empty.
       :rtype: `bool`
     */
-    proc isEmpty(): bool { return true; }
+    proc isEmpty(): bool {
+      return true;
+    }
 
+    /*
+      Returns a new DefaultRectangular array containing the elements of this
+      List.
+
+      :return: A new DefaultRectangular array.
+    */
+    proc toArray(): this._etype[] {
+      return nil;
+    }
   }
 
   proc =(a: List(?t), b: List(t)) {}
 
-  proc ==(a: List(?t), b: List(t)): bool { return true; }
+  /*
+    Returns `true` if the contents of two Lists are the same.
+
+    :arg a: A List to compare.
+    :arg b: A List to compare.
+
+    :return: `true` if the contents of two Lists are the same.
+    :rtype: `bool`
+  */
+  proc ==(a: List(?t), b: List(t)): bool {
+    return true;
+  }
 
   /*
     An important question here: do we want the semantics of addition to mirror
@@ -316,7 +351,14 @@ module Lists {
 
     Given Python semantics, `+` would be a shorthand for concatenating two
     Lists together, returning a new List as a result.
+
+    :arg a: A List to add.
+    :arg b: A List to add.
+
+    :return: A new List whose elements are the concatenation of two Lists.
   */
   proc +(a: List(?t), b: List(t)): List(t) { return nil; }
 
-}
+
+} // End module Lists!
+

From bf60ce6dff787807bba67c85c34344f167d027ff Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Wed, 15 May 2019 14:56:59 -0700
Subject: [PATCH 04/11] Fix partial tag in docstring

---
 modules/standard/Lists.chpl | 1 -
 1 file changed, 1 deletion(-)

diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
index ce2971c59278..7c4ba2fea0b2 100644
--- a/modules/standard/Lists.chpl
+++ b/modules/standard/Lists.chpl
@@ -271,7 +271,6 @@ module Lists {
       Produce a serial iterator over the elements of this List.
 
       :yields: A reference to one of the elements contained in this List.
-      :y
     */
     iter these() ref {
       yield _size;

From ec53952b4d9adbcf3d9760d8b5d12d37ae16a398 Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Wed, 15 May 2019 15:36:14 -0700
Subject: [PATCH 05/11] Add modules/standard/Lists.chpl to chpldoc coverage

---
 modules/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/modules/Makefile b/modules/Makefile
index 18b4896e9596..d25d53a35794 100644
--- a/modules/Makefile
+++ b/modules/Makefile
@@ -78,6 +78,7 @@ MODULES_TO_DOCUMENT = \
 	standard/IO.chpl \
 	standard/LinkedLists.chpl \
 	standard/List.chpl \
+  standard/Lists.chpl \
 	standard/Math.chpl \
 	standard/Memory.chpl \
 	standard/Path.chpl \

From 67a33958cbfbe4fe2eae6d45e8e4f44297749580 Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Wed, 15 May 2019 15:41:57 -0700
Subject: [PATCH 06/11] Fix soft tab in Makefile and add nodoc pragma to deinit

---
 modules/Makefile            | 2 +-
 modules/standard/Lists.chpl | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/modules/Makefile b/modules/Makefile
index d25d53a35794..dc881f629bce 100644
--- a/modules/Makefile
+++ b/modules/Makefile
@@ -78,7 +78,7 @@ MODULES_TO_DOCUMENT = \
 	standard/IO.chpl \
 	standard/LinkedLists.chpl \
 	standard/List.chpl \
-  standard/Lists.chpl \
+	standard/Lists.chpl \
 	standard/Math.chpl \
 	standard/Memory.chpl \
 	standard/Path.chpl \
diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
index 7c4ba2fea0b2..333e0dc1b63b 100644
--- a/modules/standard/Lists.chpl
+++ b/modules/standard/Lists.chpl
@@ -141,7 +141,8 @@ module Lists {
     proc init=(other: List) {
       this._etype = other._etype;
     }
-  
+
+    pragma "no doc"
     proc deinit() {}
 
     /*

From d9c3433cbd9a7b26b9eb9f187d7e8920d45dba19 Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Wed, 15 May 2019 15:53:29 -0700
Subject: [PATCH 07/11] Fix bad array subscripts

---
 modules/standard/Lists.chpl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
index 333e0dc1b63b..ecee076139eb 100644
--- a/modules/standard/Lists.chpl
+++ b/modules/standard/Lists.chpl
@@ -169,7 +169,7 @@ module Lists {
 
       :arg other: An array, the elements of which are appended to this List.
     */
-    proc extend(other: this._etype[?d]) {}
+    proc extend(other: [?d] this._etype) {}
 
     /*
       Insert an element at a given position in this List, shifting all elements
@@ -321,7 +321,7 @@ module Lists {
 
       :return: A new DefaultRectangular array.
     */
-    proc toArray(): this._etype[] {
+    proc toArray(): [] this._etype {
       return nil;
     }
   }

From 8e838677c5c2dda505cd7bf2fd2c21e1f8080fa8 Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Thu, 16 May 2019 14:32:45 -0700
Subject: [PATCH 08/11] Adjust docstrings and remove methods in response to
 feedback

---
 modules/standard/Lists.chpl | 258 ++++++++++++++++++------------------
 1 file changed, 132 insertions(+), 126 deletions(-)

diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
index ecee076139eb..41c45c836b5e 100644
--- a/modules/standard/Lists.chpl
+++ b/modules/standard/Lists.chpl
@@ -22,43 +22,41 @@
 
   ---
 
-  This module contains the implementation of a list datatype, intended to
-  replace the use of arrays to perform "vector-like" operations.
+  This module contains the implementation of the List type.
 
-  A List is a lightweight container intended to mimic Python's list datatype,
-  with fast O(log n)/O(1) (whatever we end up on) indexing  and an API close
-  in spirit to its Python counterpart.
+  A List is a lightweight container similar to an array that is suitable for
+  building up and iterating over a collection of elements in a structured
+  manner.
 
   The highly parallel nature of Chapel means that great care should be taken
-  when performing operations which may trigger a move of List elements.
-  Inserts and removals into the middle of a List are example operations which
-  may trigger a move, thus invalidating all references to elements of the
-  List held by tasks before the move. Appending an element to the end of a
-  List will never cause a move.
-
-  The following operations may trigger a move:
-    - insert
-    - remove
-    - reverse
-    - sort
-    - pop
+  when performing operations that may invalidate references to List elements.
+  Inserts and removals into the middle of a List are example operations that
+  may invalidate references. Appending an element to the end of a List will
+  never invalidate references to elements contained in the List.
 
-  ---
+  The following operations may invalidate references to elements contained in
+  a List:
 
-  (End word smithing module text).
+      - insert
+      - remove
+      - reverse
+      - sort
+      - pop
 
-  I know that @mppf in particular is very interested in limiting the number of
-  operations that move/displace List elements.
+  Additionally, all references to List elements are invalidated when the List
+  is deinitialized.
 
-  This pertains to the general issue of invalidating references to (and also
-  iterators over) List elements.
+  (Paragraph about parallel safety).
 
-  There is also the separate, more general issue of parallel safety. I'd
-  happily appreciate any/all recommendations for the "Chapeltastic" way of
-  making this List type parallel safe.
+  Inserts and removals into a List are O(n) worst case and should be performed
+  with care. For fast O(1) appending to either end consider the use of the
+  Deque type instead. For O(1) inserts and removals during iteration, consider
+  the use of the LinkedList type.
 
   ---
 
+  (This text will NOT appear in final module description).
+
   I believe it is the intention of this new List datatype to replace all
   "array-as-vec" operations on arrays. That means the following procedures
   would no longer be supported (deprecated?, or removed?) on arrays:
@@ -90,28 +88,29 @@ module Lists {
   }
 
   /*
-    A container intended to replace the use of arrays to perform "vector-like"
-    operations.
-
-    Is not currently parallel safe.
-
-    The following operations may trigger a move:
-      - insert
-      - remove
-      - reverse
-      - sort
-      - pop
-
-    A move will invalidate any references to List elements held by tasks, as
-    well as iterators to List elements produced by tasks before the move
-    occurred.
+    A List is a lightweight container suitable for building up and iterating
+    over a collection of elements in a structured manner. It is intended to
+    replace the use of arrays to perform "vector-like" operations. Unlike a
+    stack, the List type also supports inserts or removals into the middle of
+    the List. The List type is close in spirit to its Python counterpart, with
+    fast O(log n) (and O(1) eventually) indexing.
+
+    The List type is parallel safe by default. For situations in which such
+    overhead is undesirable, parallel safety can be disabled by setting
+    `parSafe = false` in the List constructor. A List object constructed
+    from another List object inherits the parallel safety mode of that list
+    unless otherwise specified.
   */
   class List {
 
-    pragma "no doc"
-    var _size = 0;
-    pragma "no doc"
-    var _etype;
+    /* The number of elements contained in this List. */
+    var size = 0;
+
+    /* The type of the elements contained in this List. */
+    type eltType;
+
+    /* If `true`, this List will use parallel safe operations. */
+    param parSafe = true;
 
     //
     // Michael suggested that we move from a LL of blocks to an array, which
@@ -122,99 +121,129 @@ module Lists {
     // indexing scheme when everything else is working.
     //
     pragma "no doc"
-    var _head, _tail: unmanaged ListBlock(_etype) = nil;
+    var _head, _tail: unmanaged ListBlock(eltType) = nil;
 
     /*
-      Initializes a new List containing elements of the given type.
+      Initializes an empty List containing elements of the given type.
 
-      :arg etype: The type of the elements of this List.
+      :arg eltType: The type of the elements of this List.
+      :arg parSafe: If `true`, this List will use parallel safe operations.
+        Is `true` by default.
     */
-    proc init(type etype) {
-      _etype = etype;
+    proc init(type eltType, param parSafe=true) {
+      this.eltType = eltType;
     }
 
     /*
-      Initializes a new List as a copy of another List object.
+      Initializes a List containing elements that are copy initialized from
+      the elements in the old List.
 
       :arg other: The List object to initialize from.
+      :arg parSafe: If `true`, this List will use parallel safe operations.
+        Inherits the value from the other List by default.
     */
-    proc init=(other: List) {
-      this._etype = other._etype;
+    proc init=(other: List, param parSafe=other.parSafe) {
+      this.eltType = other.eltType;
     }
 
     pragma "no doc"
     proc deinit() {}
 
+    // This may not be necessary, so don't fixate too much on it.
+    pragma "no doc"
+    proc _cast(type t: string, x: List): string {
+      return "";
+    }
+
     /*
       Add an element to the end of this List.
 
-      .. note::
-
-        Appending an element to a List will never invalidate references to
-        elements in the List.
-
       :arg x: An element to append.
     */
-    proc append(x: this._etype) {}
+    proc append(in x: eltType) {}
 
     /*
-      Extend this List by appending all the elements from another List.
+      Extend this List by appending a copy of each element contained in
+      another List.
 
-      :arg other: A List, the elements of which are appended to this List.
+      :arg other: A List containing elements of the same type as those
+        contained in this List.
     */
-    proc extend(other: List(this._etype)) {}
+    proc extend(other: List(eltType)) {}
 
     /*
-      Extend this List by appending all the elements from an array.
+      Extend this List by appending a copy of each element contained in an
+      array.
 
-      :arg other: An array, the elements of which are appended to this List.
+      :arg other: An array containing elements of the same type as those
+        contained in this List.
     */
-    proc extend(other: [?d] this._etype) {}
+    proc extend(other: [?d] eltType) {}
 
     /*
       Insert an element at a given position in this List, shifting all elements
-      following that index one to the right. Note that `a.insert(a.size, x)`
-      is equivalent to `a.append(x)`.
+      following that index one to the right.
+
+      .. warn::
+      
+        Inserting an element into this List may invalidate existing references
+        to the elements contained in this List.
 
       :arg i: The index of the element at which to insert.
       :arg x: The element to insert.
 
       :throws IllegalArgumentError: If the given index is out of bounds.
     */
-    proc insert(i: int, x: this._etype) throws {}
+    proc insert(i: int, in x: eltType) throws {}
 
     /*
-      Remove the first item from this List whose value is equal to _x_.
+      Remove the first item from this List whose value is equal to x, shifting
+      all elements following the removed item one to the left.
+
+      .. warn::
+
+        Removing an element from this List may invalidate existing references
+        to the elements contained in this List.
 
       :arg x: The element to remove.
 
       :throws IllegalArgumentError: If there is no such item.
     */
-    proc remove(x: this._etype) throws {}
+    proc remove(x: eltType) throws {}
 
     /*
       Remove the item at the given position in this List, and return it. If no
       index is specified, remove and return the last item in this List.
 
+      .. warn::
+
+        Popping an element from this List will invalidate any reference to
+        the element taken while it was contained in this List.
+
       :arg i: The index of the element to remove. Defaults to the last item
         in this List, or `a.remove(a.size - 1)`.
-      
+
       :return: The item removed.
 
       :throws IllegalArgumentError: If the given index is out of bounds.
     */
-    proc pop(i: int=this._size - 1): this._etype throws {
+    proc pop(i: int=size - 1): eltType throws {
       return nil;
     }
 
     /*
       Clear the contents of this List.
+
+      .. warn::
+
+        Clearing the contents of this List will invalidate all existing
+        references to the elements contained in this List.
     */
     proc clear() {}
 
     /*
       Return a zero-based index into this List of the first item whose value
-      is equal to _x_.
+      is equal to x.
 
       :arg x: An element to search for.
 
@@ -222,7 +251,7 @@ module Lists {
 
       :throws IllegalArgumentError: If the given element cannot be found.
     */
-    proc indexOf(x: _etype, start: int=0, end: int=_size): int throws {
+    proc indexOf(x: eltType, start: int=0, end: int=size): int throws {
       return 0; 
     }
 
@@ -234,25 +263,32 @@ module Lists {
       :return: The number of times a given element is found in this List.
       :rtype: `int`
     */
-    proc count(x: this._etype): int {
+    proc count(x: eltType): int {
       return 0;
     }
 
     /*
-      Sort the items of this List in place. The parameters of this method are
-      holdovers from Python. This method could more closely integrate with
-      Chapel sorting APIs.
+       Sort the elements of this List in place using their default sort order.
 
-      :arg key: Unused in current API.
-      :arg reverse: True if this List should be sorted in reverse order.
+       .. warn::
 
+        Sorting the contents of this List may invalidate existing references
+        to the elements contained in this List.
     */
-    proc sort(key: this._etype=nil, reverse: bool=false) {}
+    proc sort() {}
 
     /*
-      Reverse the elements of this List in place.
+      Sort the items of this List in place using a comparator.
+
+      .. warn::
+
+        Sorting the elements of this List may invalidate existing references
+        to the elements contained in this List.
+
+      :arg comparator: A comparator object used to sort this List.
+
     */
-    proc reverse() {}
+    proc sort(comparator) {}
 
     /*
       Index this List via subscript. Returns a reference to the element at a
@@ -274,7 +310,7 @@ module Lists {
       :yields: A reference to one of the elements contained in this List.
     */
     iter these() ref {
-      yield _size;
+      yield size;
     }
 
     /*
@@ -284,27 +320,6 @@ module Lists {
     */
     proc writeThis(ch: channel) {}
 
-    /*
-      Convert the contents of this List to string form. Could be replaced by
-      an overload of the cast operation?
-
-      :return: The contents of this List, in a form suitable for printing.
-      :rtype: `string`
-    */
-    proc toString(): string {
-      return "";
-    }
-
-    /*
-      Return the number of elements in this List.
-
-      :return: The number of elements in this List.
-      :rtype: `int`
-    */
-    proc size {
-      return _size;
-    }
-
     /*
       Returns `true` if this List contains zero elements.
 
@@ -321,11 +336,21 @@ module Lists {
 
       :return: A new DefaultRectangular array.
     */
-    proc toArray(): [] this._etype {
+    proc toArray(): [] eltType {
       return nil;
     }
-  }
 
+  } // End class List!
+
+  /*
+    (This is a question for debate, not an actual docstring).
+
+    Should the List type support a deep copy on assignment (IE, an overload of
+    the `=` operator along with making it a record)?
+
+    IE, when I assign L2 to L1, the contents of L1 are now a copy of the
+    elements contained in L2.
+  */
   proc =(a: List(?t), b: List(t)) {}
 
   /*
@@ -341,24 +366,5 @@ module Lists {
     return true;
   }
 
-  /*
-    An important question here: do we want the semantics of addition to mirror
-    Chapel arrays, or Python lists?
-
-    Given Chapel semantics, `+` would be a "vectorlike" operation, performing
-    scalar addition against all elements of this List, returning a new List
-    as a result.
-
-    Given Python semantics, `+` would be a shorthand for concatenating two
-    Lists together, returning a new List as a result.
-
-    :arg a: A List to add.
-    :arg b: A List to add.
-
-    :return: A new List whose elements are the concatenation of two Lists.
-  */
-  proc +(a: List(?t), b: List(t)): List(t) { return nil; }
-
-
 } // End module Lists!
 

From 5bbd4e5aedebdca905d16d1ad5386efe90c83d4e Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Thu, 16 May 2019 14:50:09 -0700
Subject: [PATCH 09/11] Fix typo in docstring

---
 modules/standard/Lists.chpl | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
index 41c45c836b5e..ad2ff538fba4 100644
--- a/modules/standard/Lists.chpl
+++ b/modules/standard/Lists.chpl
@@ -221,7 +221,7 @@ module Lists {
         the element taken while it was contained in this List.
 
       :arg i: The index of the element to remove. Defaults to the last item
-        in this List, or `a.remove(a.size - 1)`.
+        in this List.
 
       :return: The item removed.
 
@@ -256,7 +256,7 @@ module Lists {
     }
 
     /*
-      Return the number of times _x_ appears in this List.
+      Return the number of times a given element is found in this List.
 
       :arg x: An element to count.
 
@@ -314,7 +314,7 @@ module Lists {
     }
 
     /*
-      Write the contents of this List to a given channel.
+      Write the contents of this List to a channel.
 
       :arg ch: A channel to write to.
     */
@@ -331,8 +331,8 @@ module Lists {
     }
 
     /*
-      Returns a new DefaultRectangular array containing the elements of this
-      List.
+      Returns a new DefaultRectangular array containing a copy of each of the
+      elements contained in this List.
 
       :return: A new DefaultRectangular array.
     */

From 17e7d7850124650f8809b07c479783a6b37284c4 Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Thu, 16 May 2019 14:53:21 -0700
Subject: [PATCH 10/11] Bad doc tag, change warn -> warning

---
 modules/standard/Lists.chpl | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
index ad2ff538fba4..0333b03570b9 100644
--- a/modules/standard/Lists.chpl
+++ b/modules/standard/Lists.chpl
@@ -184,7 +184,7 @@ module Lists {
       Insert an element at a given position in this List, shifting all elements
       following that index one to the right.
 
-      .. warn::
+      .. warning::
       
         Inserting an element into this List may invalidate existing references
         to the elements contained in this List.
@@ -200,7 +200,7 @@ module Lists {
       Remove the first item from this List whose value is equal to x, shifting
       all elements following the removed item one to the left.
 
-      .. warn::
+      .. warning::
 
         Removing an element from this List may invalidate existing references
         to the elements contained in this List.
@@ -215,7 +215,7 @@ module Lists {
       Remove the item at the given position in this List, and return it. If no
       index is specified, remove and return the last item in this List.
 
-      .. warn::
+      .. warning::
 
         Popping an element from this List will invalidate any reference to
         the element taken while it was contained in this List.
@@ -234,7 +234,7 @@ module Lists {
     /*
       Clear the contents of this List.
 
-      .. warn::
+      .. warning::
 
         Clearing the contents of this List will invalidate all existing
         references to the elements contained in this List.
@@ -270,7 +270,7 @@ module Lists {
     /*
        Sort the elements of this List in place using their default sort order.
 
-       .. warn::
+       .. warning::
 
         Sorting the contents of this List may invalidate existing references
         to the elements contained in this List.
@@ -280,7 +280,7 @@ module Lists {
     /*
       Sort the items of this List in place using a comparator.
 
-      .. warn::
+      .. warning::
 
         Sorting the elements of this List may invalidate existing references
         to the elements contained in this List.

From a8a81bfc91ad6e2ea10e935802d29bf3f67570b6 Mon Sep 17 00:00:00 2001
From: David Longnecker <dlongnecke@cray.com>
Date: Thu, 16 May 2019 16:19:50 -0700
Subject: [PATCH 11/11] Change List to be a record instead of a class

---
 modules/standard/Lists.chpl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/standard/Lists.chpl b/modules/standard/Lists.chpl
index 0333b03570b9..ffea929a4129 100644
--- a/modules/standard/Lists.chpl
+++ b/modules/standard/Lists.chpl
@@ -101,7 +101,7 @@ module Lists {
     from another List object inherits the parallel safety mode of that list
     unless otherwise specified.
   */
-  class List {
+  record List {
 
     /* The number of elements contained in this List. */
     var size = 0;