2011-11-23 Ed Schonberg <schonberg@adacore.com>

	* exp_ch5.adb (Expand_Iterator_Loop): Wrap the expanded loop
	and the cursor declarations in a block, so that the loop variable
	is local to the construct.

2011-11-23  Matthew Heaney  <heaney@adacore.com>

	* a-coorma.ads, a-ciorma.ads, a-cborma.ads (Iterate): Returns
	type Reversible_Iterator'Class.
	* a-coorma.adb, a-ciorma.adb, a-cborma.adb (Iterator):
	Declare type as limited.
	(First, Last): Return value depends on iterator's start node value.
	(Next, Previous): Call corresponding Cursor-based operation.
	(Iterate): Indicate whether complete or partial iteration



git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@181659 138bc75d-0d04-0410-961f-82ee72b054a4

8 files changed, 368 insertions, 93 deletions

diff --git a/gcc/ada/ChangeLog b/gcc/ada/ChangeLog
index 1f574e2246c..30486c131c5 100644
--- a/gcc/ada/ChangeLog
+++ b/gcc/ada/ChangeLog
@@ -1,3 +1,19 @@
+2011-11-23  Ed Schonberg  <schonberg@adacore.com>
+
+	* exp_ch5.adb (Expand_Iterator_Loop): Wrap the expanded loop
+	and the cursor declarations in a block, so that the loop variable
+	is local to the construct.
+
+2011-11-23  Matthew Heaney  <heaney@adacore.com>
+
+	* a-coorma.ads, a-ciorma.ads, a-cborma.ads (Iterate): Returns
+	type Reversible_Iterator'Class.
+	* a-coorma.adb, a-ciorma.adb, a-cborma.adb (Iterator):
+	Declare type as limited.
+	(First, Last): Return value depends on iterator's start node value.
+	(Next, Previous): Call corresponding Cursor-based operation.
+	(Iterate): Indicate whether complete or partial iteration
+
 2011-11-23  Robert Dewar  <dewar@adacore.com>
 
 	* errout.adb: Minor reformattin (Finalize): Take templates into
diff --git a/gcc/ada/a-cborma.adb b/gcc/ada/a-cborma.adb
index 4cc2686bb22..940d6efa9cb 100644
--- a/gcc/ada/a-cborma.adb
+++ b/gcc/ada/a-cborma.adb
@@ -39,7 +39,7 @@ with System;  use type System.Address;
 
 package body Ada.Containers.Bounded_Ordered_Maps is
 
-   type Iterator is new
+   type Iterator is limited new
      Map_Iterator_Interfaces.Reversible_Iterator with record
         Container : Map_Access;
         Node      : Count_Type;
@@ -579,12 +579,24 @@ package body Ada.Containers.Bounded_Ordered_Maps is
    end First;
 
    function First (Object : Iterator) return Cursor is
-      F : constant Count_Type := Object.Container.First;
    begin
-      if F = 0 then
-         return No_Element;
+      --  The value of the iterator object's Node component influences the
+      --  behavior of the First (and Last) selector function.
+
+      --  When the Node component is 0, this means the iterator object was
+      --  constructed without a start expression, in which case the (forward)
+      --  iteration starts from the (logical) beginning of the entire sequence
+      --  of items (corresponding to Container.First, for a forward iterator).
+
+      --  Otherwise, this is iteration over a partial sequence of items. When
+      --  the Node component is positive, the iterator object was constructed
+      --  with a start expression, that specifies the position from which the
+      --  (forward) partial iteration begins.
+
+      if Object.Node = 0 then
+         return Bounded_Ordered_Maps.First (Object.Container.all);
       else
-         return Cursor'(Object.Container.all'Unchecked_Access, F);
+         return Cursor'(Object.Container, Object.Node);
       end if;
    end First;
 
@@ -886,22 +898,62 @@ package body Ada.Containers.Bounded_Ordered_Maps is
    end Iterate;
 
    function Iterate
-     (Container : Map) return Map_Iterator_Interfaces.Forward_Iterator'class
+     (Container : Map) return Map_Iterator_Interfaces.Reversible_Iterator'Class
    is
-      It : constant Iterator :=
-             (Container'Unrestricted_Access, Container.First);
    begin
-      return It;
+      --  The value of the Node component influences the behavior of the First
+      --  and Last selector functions of the iterator object. When the Node
+      --  component is 0 (as is the case here), this means the iterator object
+      --  was constructed without a start expression. This is a complete
+      --  iterator, meaning that the iteration starts from the (logical)
+      --  beginning of the sequence of items.
+
+      --  Note: For a forward iterator, Container.First is the beginning, and
+      --  for a reverse iterator, Container.Last is the beginning.
+
+      return Iterator'(Container'Unrestricted_Access, Node => 0);
    end Iterate;
 
    function Iterate
      (Container : Map;
       Start     : Cursor)
-      return Map_Iterator_Interfaces.Reversible_Iterator'class
+      return Map_Iterator_Interfaces.Reversible_Iterator'Class
    is
-      It : constant Iterator := (Container'Unrestricted_Access, Start.Node);
    begin
-      return It;
+
+      --  iterator was defined to behave the same as for a complete iterator,
+      --  and iterate over the entire sequence of items. However, those
+      --  semantics were unintuitive and arguably error-prone (it is too easy
+      --  to accidentally create an endless loop), and so they were changed,
+      --  per the ARG meeting in Denver on 2011/11. However, there was no
+      --  consensus about what positive meaning this corner case should have,
+      --  and so it was decided to simply raise an exception. This does imply,
+      --  however, that it is not possible to use a partial iterator to specify
+      --  an empty sequence of items.
+
+      if Start = No_Element then
+         raise Constraint_Error with
+           "Start position for iterator equals No_Element";
+      end if;
+
+      if Start.Container /= Container'Unrestricted_Access then
+         raise Program_Error with
+           "Start cursor of Iterate designates wrong map";
+      end if;
+
+      pragma Assert (Vet (Container, Start.Node),
+                     "Start cursor of Iterate is bad");
+
+      --  The value of the Node component influences the behavior of the First
+      --  and Last selector functions of the iterator object. When the Node
+      --  component is positive (as is the case here), it means that this
+      --  is a partial iteration, over a subset of the complete sequence of
+      --  items. The iterator object was constructed with a start expression,
+      --  indicating the position from which the iteration begins. (Note that
+      --  the start position has the same value irrespective of whether this
+      --  is a forward or reverse iteration.)
+
+      return Iterator'(Container'Unrestricted_Access, Node => Start.Node);
    end Iterate;
 
    ---------
@@ -935,12 +987,24 @@ package body Ada.Containers.Bounded_Ordered_Maps is
    end Last;
 
    function Last (Object : Iterator) return Cursor is
-      F : constant Count_Type := Object.Container.Last;
    begin
-      if F = 0 then
-         return No_Element;
+      --  The value of the iterator object's Node component influences the
+      --  behavior of the Last (and First) selector function.
+
+      --  When the Node component is 0, this means the iterator object was
+      --  constructed without a start expression, in which case the (reverse)
+      --  iteration starts from the (logical) beginning of the entire sequence
+      --  (corresponding to Container.Last, for a reverse iterator).
+
+      --  Otherwise, this is iteration over a partial sequence of items. When
+      --  the Node component is positive, the iterator object was constructed
+      --  with a start expression, that specifies the position from which the
+      --  (reverse) partial iteration begins.
+
+      if Object.Node = 0 then
+         return Bounded_Ordered_Maps.Last (Object.Container.all);
       else
-         return Cursor'(Object.Container.all'Unchecked_Access, F);
+         return Cursor'(Object.Container, Object.Node);
       end if;
    end Last;
 
@@ -1044,8 +1108,16 @@ package body Ada.Containers.Bounded_Ordered_Maps is
      (Object   : Iterator;
       Position : Cursor) return Cursor
    is
-      pragma Unreferenced (Object);
    begin
+      if Position.Container = null then
+         return No_Element;
+      end if;
+
+      if Position.Container /= Object.Container then
+         raise Program_Error with
+           "Position cursor of Next designates wrong map";
+      end if;
+
       return Next (Position);
    end Next;
 
@@ -1095,8 +1167,16 @@ package body Ada.Containers.Bounded_Ordered_Maps is
      (Object   : Iterator;
       Position : Cursor) return Cursor
    is
-      pragma Unreferenced (Object);
    begin
+      if Position.Container = null then
+         return No_Element;
+      end if;
+
+      if Position.Container /= Object.Container then
+         raise Program_Error with
+           "Position cursor of Previous designates wrong map";
+      end if;
+
       return Previous (Position);
    end Previous;
 
diff --git a/gcc/ada/a-cborma.ads b/gcc/ada/a-cborma.ads
index e1f9f08f379..05c55730f10 100644
--- a/gcc/ada/a-cborma.ads
+++ b/gcc/ada/a-cborma.ads
@@ -227,17 +227,18 @@ package Ada.Containers.Bounded_Ordered_Maps is
      (Container : Map;
       Process   : not null access procedure (Position : Cursor));
 
+   procedure Reverse_Iterate
+     (Container : Map;
+      Process   : not null access procedure (Position : Cursor));
+
    function Iterate
-     (Container : Map) return Map_Iterator_Interfaces.Forward_Iterator'class;
+     (Container : Map)
+      return Map_Iterator_Interfaces.Reversible_Iterator'Class;
 
    function Iterate
      (Container : Map;
       Start     : Cursor)
-      return Map_Iterator_Interfaces.Reversible_Iterator'class;
-
-   procedure Reverse_Iterate
-     (Container : Map;
-      Process   : not null access procedure (Position : Cursor));
+      return Map_Iterator_Interfaces.Reversible_Iterator'Class;
 
 private
 
diff --git a/gcc/ada/a-ciorma.adb b/gcc/ada/a-ciorma.adb
index cd95b9fd5ab..ea8fa75636b 100644
--- a/gcc/ada/a-ciorma.adb
+++ b/gcc/ada/a-ciorma.adb
@@ -40,7 +40,7 @@ with System; use type System.Address;
 package body Ada.Containers.Indefinite_Ordered_Maps is
    pragma Suppress (All_Checks);
 
-   type Iterator is new
+   type Iterator is limited new
      Map_Iterator_Interfaces.Reversible_Iterator with record
         Container : Map_Access;
         Node      : Node_Access;
@@ -558,11 +558,25 @@ package body Ada.Containers.Indefinite_Ordered_Maps is
    end First;
 
    function First (Object : Iterator) return Cursor is
-      M : constant Map_Access  := Object.Container;
-      N : constant Node_Access := M.Tree.First;
    begin
-      return (if N = null then No_Element
-              else Cursor'(Object.Container.all'Unchecked_Access, N));
+      --  The value of the iterator object's Node component influences the
+      --  behavior of the First (and Last) selector function.
+
+      --  When the Node component is null, this means the iterator object was
+      --  constructed without a start expression, in which case the (forward)
+      --  iteration starts from the (logical) beginning of the entire sequence
+      --  of items (corresponding to Container.First for a forward iterator).
+
+      --  Otherwise, this is iteration over a partial sequence of items. When
+      --  the Node component is non-null, the iterator object was constructed
+      --  with a start expression, that specifies the position from which the
+      --  (forward) partial iteration begins.
+
+      if Object.Node = null then
+         return Object.Container.First;
+      else
+         return Cursor'(Object.Container, Object.Node);
+      end if;
    end First;
 
    -------------------
@@ -571,13 +585,12 @@ package body Ada.Containers.Indefinite_Ordered_Maps is
 
    function First_Element (Container : Map) return Element_Type is
       T : Tree_Type renames Container.Tree;
-
    begin
       if T.First = null then
          raise Constraint_Error with "map is empty";
+      else
+         return T.First.Element.all;
       end if;
-
-      return T.First.Element.all;
    end First_Element;
 
    ---------------
@@ -586,13 +599,12 @@ package body Ada.Containers.Indefinite_Ordered_Maps is
 
    function First_Key (Container : Map) return Key_Type is
       T : Tree_Type renames Container.Tree;
-
    begin
       if T.First = null then
          raise Constraint_Error with "map is empty";
+      else
+         return T.First.Key.all;
       end if;
-
-      return T.First.Key.all;
    end First_Key;
 
    -----------
@@ -864,22 +876,62 @@ package body Ada.Containers.Indefinite_Ordered_Maps is
    end Iterate;
 
    function Iterate
-     (Container : Map) return Map_Iterator_Interfaces.Forward_Iterator'class
+     (Container : Map) return Map_Iterator_Interfaces.Reversible_Iterator'Class
    is
-      Node : constant Node_Access := Container.Tree.First;
-      It   : constant Iterator := (Container'Unrestricted_Access, Node);
    begin
-      return It;
+      --  The value of the Node component influences the behavior of the First
+      --  and Last selector functions of the iterator object. When the Node
+      --  component is null (as is the case here), this means the iterator
+      --  object was constructed without a start expression. This is a complete
+      --  iterator, meaning that the iteration starts from the (logical)
+      --  beginning of the sequence of items.
+
+      --  Note: For a forward iterator, Container.First is the beginning, and
+      --  for a reverse iterator, Container.Last is the beginning.
+
+      return Iterator'(Container'Unrestricted_Access, Node => null);
    end Iterate;
 
    function Iterate
      (Container : Map;
       Start     : Cursor)
-      return Map_Iterator_Interfaces.Reversible_Iterator'class
+      return Map_Iterator_Interfaces.Reversible_Iterator'Class
    is
-      It : constant Iterator := (Container'Unrestricted_Access, Start.Node);
    begin
-      return It;
+      --  It was formerly the case that when Start = No_Element, the partial
+      --  iterator was defined to behave the same as for a complete iterator,
+      --  and iterate over the entire sequence of items. However, those
+      --  semantics were unintuitive and arguably error-prone (it is too easy
+      --  to accidentally create an endless loop), and so they were changed,
+      --  per the ARG meeting in Denver on 2011/11. However, there was no
+      --  consensus about what positive meaning this corner case should have,
+      --  and so it was decided to simply raise an exception. This does imply,
+      --  however, that it is not possible to use a partial iterator to specify
+      --  an empty sequence of items.
+
+      if Start = No_Element then
+         raise Constraint_Error with
+           "Start position for iterator equals No_Element";
+      end if;
+
+      if Start.Container /= Container'Unrestricted_Access then
+         raise Program_Error with
+           "Start cursor of Iterate designates wrong map";
+      end if;
+
+      pragma Assert (Vet (Container.Tree, Start.Node),
+                     "Start cursor of Iterate is bad");
+
+      --  The value of the Node component influences the behavior of the First
+      --  and Last selector functions of the iterator object. When the Node
+      --  component is non-null (as is the case here), it means that this
+      --  is a partial iteration, over a subset of the complete sequence of
+      --  items. The iterator object was constructed with a start expression,
+      --  indicating the position from which the iteration begins. Note that
+      --  the start position has the same value irrespective of whether this
+      --  is a forward or reverse iteration.
+
+      return Iterator'(Container'Unrestricted_Access, Node => Start.Node);
    end Iterate;
 
    ---------
@@ -916,11 +968,25 @@ package body Ada.Containers.Indefinite_Ordered_Maps is
    end Last;
 
    function Last (Object : Iterator) return Cursor is
-      M : constant Map_Access  := Object.Container;
-      N : constant Node_Access := M.Tree.Last;
    begin
-      return (if N = null then No_Element
-              else Cursor'(Object.Container.all'Unchecked_Access, N));
+      --  The value of the iterator object's Node component influences the
+      --  behavior of the Last (and First) selector function.
+
+      --  When the Node component is null, this means the iterator object was
+      --  constructed without a start expression, in which case the (reverse)
+      --  iteration starts from the (logical) beginning of the entire sequence
+      --  (corresponding to Container.Last, for a reverse iterator).
+
+      --  Otherwise, this is iteration over a partial sequence of items. When
+      --  the Node component is non-null, the iterator object was constructed
+      --  with a start expression, that specifies the position from which the
+      --  (reverse) partial iteration begins.
+
+      if Object.Node = null then
+         return Object.Container.Last;
+      else
+         return Cursor'(Object.Container, Object.Node);
+      end if;
    end Last;
 
    ------------------
@@ -1017,8 +1083,16 @@ package body Ada.Containers.Indefinite_Ordered_Maps is
       Position : Cursor) return Cursor
    is
    begin
-      return (if Position.Node = null then No_Element
-              else (Object.Container, Tree_Operations.Next (Position.Node)));
+      if Position.Container = null then
+         return No_Element;
+      end if;
+
+      if Position.Container /= Object.Container then
+         raise Program_Error with
+           "Position cursor of Next designates wrong map";
+      end if;
+
+      return Next (Position);
    end Next;
 
    ------------
@@ -1065,9 +1139,16 @@ package body Ada.Containers.Indefinite_Ordered_Maps is
       Position : Cursor) return Cursor
    is
    begin
-      return
-        (if Position.Node = null then No_Element
-         else (Object.Container, Tree_Operations.Previous (Position.Node)));
+      if Position.Container = null then
+         return No_Element;
+      end if;
+
+      if Position.Container /= Object.Container then
+         raise Program_Error with
+           "Position cursor of Previous designates wrong map";
+      end if;
+
+      return Previous (Position);
    end Previous;
 
    -------------------
@@ -1490,4 +1571,5 @@ package body Ada.Containers.Indefinite_Ordered_Maps is
    begin
       raise Program_Error with "attempt to stream reference";
    end Write;
+
 end Ada.Containers.Indefinite_Ordered_Maps;
diff --git a/gcc/ada/a-ciorma.ads b/gcc/ada/a-ciorma.ads
index 1c19b81161f..f4c1321835e 100644
--- a/gcc/ada/a-ciorma.ads
+++ b/gcc/ada/a-ciorma.ads
@@ -201,14 +201,18 @@ package Ada.Containers.Indefinite_Ordered_Maps is
      (Container : Map;
       Process   : not null access procedure (Position : Cursor));
 
+   --  The map container supports iteration in both the forward and reverse
+   --  directions, hence these constructor functions return an object that
+   --  supports the Reversible_Iterator interface.
+
    function Iterate
      (Container : Map)
-      return Map_Iterator_Interfaces.Forward_Iterator'class;
+      return Map_Iterator_Interfaces.Reversible_Iterator'Class;
 
    function Iterate
      (Container : Map;
       Start     : Cursor)
-      return Map_Iterator_Interfaces.Reversible_Iterator'class;
+      return Map_Iterator_Interfaces.Reversible_Iterator'Class;
 
 private
 
diff --git a/gcc/ada/a-coorma.adb b/gcc/ada/a-coorma.adb
index e8099c3c297..d5f5391d871 100644
--- a/gcc/ada/a-coorma.adb
+++ b/gcc/ada/a-coorma.adb
@@ -39,7 +39,7 @@ with System; use type System.Address;
 
 package body Ada.Containers.Ordered_Maps is
 
-   type Iterator is new
+   type Iterator is limited new
      Map_Iterator_Interfaces.Reversible_Iterator with record
         Container : Map_Access;
         Node      : Node_Access;
@@ -518,13 +518,24 @@ package body Ada.Containers.Ordered_Maps is
    end First;
 
    function First (Object : Iterator) return Cursor is
-      M : constant Map_Access  := Object.Container;
-      N : constant Node_Access := M.Tree.First;
    begin
-      if N = null then
-         return No_Element;
+      --  The value of the iterator object's Node component influences the
+      --  behavior of the First (and Last) selector function.
+
+      --  When the Node component is null, this means the iterator object was
+      --  constructed without a start expression, in which case the (forward)
+      --  iteration starts from the (logical) beginning of the entire sequence
+      --  of items (corresponding to Container.First, for a forward iterator).
+
+      --  Otherwise, this is iteration over a partial sequence of items. When
+      --  the Node component is non-null, the iterator object was constructed
+      --  with a start expression, that specifies the position from which the
+      --  (forward) partial iteration begins.
+
+      if Object.Node = null then
+         return Object.Container.First;
       else
-         return Cursor'(Object.Container.all'Unchecked_Access, N);
+         return Cursor'(Object.Container, Object.Node);
       end if;
    end First;
 
@@ -534,7 +545,6 @@ package body Ada.Containers.Ordered_Maps is
 
    function First_Element (Container : Map) return Element_Type is
       T : Tree_Type renames Container.Tree;
-
    begin
       if T.First = null then
          raise Constraint_Error with "map is empty";
@@ -827,21 +837,60 @@ package body Ada.Containers.Ordered_Maps is
    end Iterate;
 
    function Iterate
-     (Container : Map) return Map_Iterator_Interfaces.Forward_Iterator'class
+     (Container : Map) return Map_Iterator_Interfaces.Reversible_Iterator'Class
    is
-      Node : constant Node_Access := Container.Tree.First;
-      It   : constant Iterator := (Container'Unrestricted_Access, Node);
-
    begin
-      return It;
+      --  The value of the Node component influences the behavior of the First
+      --  and Last selector functions of the iterator object. When the Node
+      --  component is null (as is the case here), this means the iterator
+      --  object was constructed without a start expression. This is a
+      --  complete iterator, meaning that the iteration starts from the
+      --  (logical) beginning of the sequence of items.
+
+      --  Note: For a forward iterator, Container.First is the beginning, and
+      --  for a reverse iterator, Container.Last is the beginning.
+
+      return Iterator'(Container'Unrestricted_Access, Node => null);
    end Iterate;
 
    function Iterate (Container : Map; Start : Cursor)
-      return Map_Iterator_Interfaces.Reversible_Iterator'class
+      return Map_Iterator_Interfaces.Reversible_Iterator'Class
    is
-      It : constant Iterator := (Container'Unrestricted_Access, Start.Node);
    begin
-      return It;
+      --  It was formerly the case that when Start = No_Element, the partial
+      --  iterator was defined to behave the same as for a complete iterator,
+      --  and iterate over the entire sequence of items. However, those
+      --  semantics were unintuitive and arguably error-prone (it is too easy
+      --  to accidentally create an endless loop), and so they were changed,
+      --  per the ARG meeting in Denver on 2011/11. However, there was no
+      --  consensus about what positive meaning this corner case should have,
+      --  and so it was decided to simply raise an exception. This does imply,
+      --  however, that it is not possible to use a partial iterator to specify
+      --  an empty sequence of items.
+
+      if Start = No_Element then
+         raise Constraint_Error with
+           "Start position for iterator equals No_Element";
+      end if;
+
+      if Start.Container /= Container'Unrestricted_Access then
+         raise Program_Error with
+           "Start cursor of Iterate designates wrong map";
+      end if;
+
+      pragma Assert (Vet (Container.Tree, Start.Node),
+                     "Start cursor of Iterate is bad");
+
+      --  The value of the Node component influences the behavior of the First
+      --  and Last selector functions of the iterator object. When the Node
+      --  component is non-null (as is the case here), it means that this
+      --  is a partial iteration, over a subset of the complete sequence of
+      --  items. The iterator object was constructed with a start expression,
+      --  indicating the position from which the iteration begins. Note that
+      --  the start position has the same value irrespective of whether this
+      --  is a forward or reverse iteration.
+
+      return Iterator'(Container'Unrestricted_Access, Node => Start.Node);
    end Iterate;
 
    ---------
@@ -876,13 +925,24 @@ package body Ada.Containers.Ordered_Maps is
    end Last;
 
    function Last (Object : Iterator) return Cursor is
-      M : constant Map_Access  := Object.Container;
-      N : constant Node_Access := M.Tree.Last;
    begin
-      if N = null then
-         return No_Element;
+      --  The value of the iterator object's Node component influences the
+      --  behavior of the Last (and First) selector function.
+
+      --  When the Node component is null, this means the iterator object was
+      --  constructed without a start expression, in which case the (reverse)
+      --  iteration starts from the (logical) beginning of the entire sequence
+      --  (corresponding to Container.Last, for a reverse iterator).
+
+      --  Otherwise, this is iteration over a partial sequence of items. When
+      --  the Node component is non-null, the iterator object was constructed
+      --  with a start expression, that specifies the position from which the
+      --  (reverse) partial iteration begins.
+
+      if Object.Node = null then
+         return Object.Container.Last;
       else
-         return Cursor'(Object.Container.all'Unchecked_Access, N);
+         return Cursor'(Object.Container, Object.Node);
       end if;
    end Last;
 
@@ -980,11 +1040,16 @@ package body Ada.Containers.Ordered_Maps is
       Position : Cursor) return Cursor
    is
    begin
-      if Position.Node = null then
+      if Position.Container = null then
          return No_Element;
-      else
-         return (Object.Container, Tree_Operations.Next (Position.Node));
       end if;
+
+      if Position.Container /= Object.Container then
+         raise Program_Error with
+           "Position cursor of Next designates wrong map";
+      end if;
+
+      return Next (Position);
    end Next;
 
    ------------
@@ -1032,12 +1097,18 @@ package body Ada.Containers.Ordered_Maps is
       Position : Cursor) return Cursor
    is
    begin
-      if Position.Node = null then
+      if Position.Container = null then
          return No_Element;
-      else
-         return (Object.Container, Tree_Operations.Previous (Position.Node));
       end if;
+
+      if Position.Container /= Object.Container then
+         raise Program_Error with
+           "Position cursor of Previous designates wrong map";
+      end if;
+
+      return Previous (Position);
    end Previous;
+
    -------------------
    -- Query_Element --
    -------------------
diff --git a/gcc/ada/a-coorma.ads b/gcc/ada/a-coorma.ads
index 53942b71fa2..9d2737a5efb 100644
--- a/gcc/ada/a-coorma.ads
+++ b/gcc/ada/a-coorma.ads
@@ -203,19 +203,23 @@ package Ada.Containers.Ordered_Maps is
      (Container : Map;
       Process   : not null access procedure (Position : Cursor));
 
+   procedure Reverse_Iterate
+     (Container : Map;
+      Process   : not null access procedure (Position : Cursor));
+
+   --  The map container supports iteration in both the forward and reverse
+   --  directions, hence these constructor functions return an object that
+   --  supports the Reversible_Iterator interface.
+
    function Iterate
      (Container : Map)
-      return Map_Iterator_Interfaces.Forward_Iterator'class;
+      return Map_Iterator_Interfaces.Reversible_Iterator'class;
 
    function Iterate
      (Container : Map;
       Start     : Cursor)
       return Map_Iterator_Interfaces.Reversible_Iterator'class;
 
-   procedure Reverse_Iterate
-     (Container : Map;
-      Process   : not null access procedure (Position : Cursor));
-
 private
 
    pragma Inline (Next);
diff --git a/gcc/ada/exp_ch5.adb b/gcc/ada/exp_ch5.adb
index fd75b158449..772faa93216 100644
--- a/gcc/ada/exp_ch5.adb
+++ b/gcc/ada/exp_ch5.adb
@@ -3178,11 +3178,13 @@ package body Exp_Ch5 is
             --  Determine the advancement and initialization steps for the
             --  cursor.
 
-            --  Must verify that the container has a reverse iterator ???
+            --  Analysis of the expanded loop will verify that the container
+            --  has a reverse iterator.
 
             if Reverse_Present (I_Spec) then
                Name_Init := Name_Last;
                Name_Step := Name_Previous;
+
             else
                Name_Init := Name_First;
                Name_Step := Name_Next;
@@ -3251,6 +3253,7 @@ package body Exp_Ch5 is
             declare
                Decl1 : Node_Id;
                Decl2 : Node_Id;
+               Decl3 : Node_Id;
 
             begin
                Decl1 :=
@@ -3274,16 +3277,30 @@ package body Exp_Ch5 is
 
                Set_Assignment_OK (Decl2);
 
-               Insert_Actions (N, New_List (Decl1, Decl2));
-            end;
+               --  The cursor is only modified in expanded code, so it appears
+               --  as unassigned to the warning machinery. We must suppress
+               --  this spurious warning explicitly.
+
+               Decl3 :=
+                 Make_Pragma (Loc,
+                   Chars => Name_Warnings,
+                   Pragma_Argument_Associations => New_List (
+                     Make_Pragma_Argument_Association (Loc,
+                       Expression => Make_Identifier (Loc, Name_Off)),
+                     Make_Pragma_Argument_Association (Loc,
+                       Expression =>
+                         New_Occurrence_Of (Cursor, Loc))));
 
-            --  The Iterator is not modified in the source, but of course will
-            --  be updated in the generated code. Indicate that it is actually
-            --  set to prevent spurious warnings. Ditto for the Cursor, which
-            --  is modified indirectly in generated code.
+               --  The expanded loop is wrapped in a block, to make the loop
+               --  variable local.
 
-            Set_Never_Set_In_Source (Iterator, False);
-            Set_Never_Set_In_Source (Cursor, False);
+               New_Loop :=
+                 Make_Block_Statement (Loc,
+                   Declarations => New_List (Decl1, Decl2, Decl3),
+                   Handled_Statement_Sequence =>
+                     Make_Handled_Sequence_Of_Statements (Loc,
+                       Statements => New_List (New_Loop)));
+            end;
 
             --  If the range of iteration is given by a function call that
             --  returns a container, the finalization actions have been saved