[multiple changes]

2013-07-08  Javier Miranda  <miranda@adacore.com>

	* sem_ch8.adb (Save_Scope_Stack): Adding documentation.
	(Restore_Scope_Stack): Remove the elements of the list when the
	visibility of each entity is restored.

2013-07-08  Robert Dewar  <dewar@adacore.com>

	* exp_ch9.adb, sem.ads, sem_util.adb: Minor reformatting.

From-SVN: r200756

5 files changed, 50 insertions, 13 deletions

diff --git a/gcc/ada/ChangeLog b/gcc/ada/ChangeLog
index 0fa4a2e..c7f9f39 100644
--- a/gcc/ada/ChangeLog
+++ b/gcc/ada/ChangeLog
@@ -1,3 +1,13 @@
+2013-07-08  Javier Miranda  <miranda@adacore.com>
+
+	* sem_ch8.adb (Save_Scope_Stack): Adding documentation.
+	(Restore_Scope_Stack): Remove the elements of the list when the
+	visibility of each entity is restored.
+
+2013-07-08  Robert Dewar  <dewar@adacore.com>
+
+	* exp_ch9.adb, sem.ads, sem_util.adb: Minor reformatting.
+
 2013-07-08  Robert Dewar  <dewar@adacore.com>
 
 	* sem_ch8.adb, sem_ch8.ads: Minor reformatting.
diff --git a/gcc/ada/exp_ch9.adb b/gcc/ada/exp_ch9.adb
index 20a346c..59c5b2d 100644
--- a/gcc/ada/exp_ch9.adb
+++ b/gcc/ada/exp_ch9.adb
@@ -3347,7 +3347,7 @@ package body Exp_Ch9 is
             if Known_Static_Esize (Comp_Type) then
                Typ_Size := UI_To_Int (Esize (Comp_Type));
 
-            --  If the Esize (Object_Size) is unknown at compile-time, look at
+            --  If the Esize (Object_Size) is unknown at compile time, look at
             --  the RM_Size (Value_Size) since it may have been set by an
             --  explicit representation clause.
 
diff --git a/gcc/ada/sem.ads b/gcc/ada/sem.ads
index 545aadc..57d5d91 100644
--- a/gcc/ada/sem.ads
+++ b/gcc/ada/sem.ads
@@ -562,7 +562,7 @@ package Sem is
    --  Note: for integer and real literals, the analyzer sets the flag to
    --  indicate that the result is a static expression. If the expander
    --  generates a literal that does NOT correspond to a static expression,
-   --  e.g. by folding an expression whose value is known at compile-time,
+   --  e.g. by folding an expression whose value is known at compile time,
    --  but is not technically static, then the caller should reset the
    --  Is_Static_Expression flag after analyzing but before resolving.
    --
diff --git a/gcc/ada/sem_ch8.adb b/gcc/ada/sem_ch8.adb
index c0795b7..c5d2e99 100644
--- a/gcc/ada/sem_ch8.adb
+++ b/gcc/ada/sem_ch8.adb
@@ -7662,16 +7662,18 @@ package body Sem_Ch8 is
       Elmt    : Elmt_Id;
 
    begin
-      --  Restore visibility of previous scope stack, if any
+      --  Restore visibility of previous scope stack, if any, using the list
+      --  we saved (we use Remove, since this list will not be used again).
 
-      --  Should use Remove_Elmt, so that elements can be reused ???
-
-      Elmt := First_Elmt (List);
-      while Present (Elmt) loop
+      loop
+         Elmt := Last_Elmt (List);
+         exit when Elmt = No_Elmt;
          Set_Is_Immediately_Visible (Node (Elmt));
-         Next_Elmt (Elmt);
+         Remove_Last_Elmt (List);
       end loop;
 
+      --  Restore use clauses
+
       if SS_Last >= Scope_Stack.First
         and then Scope_Stack.Table (SS_Last).Entity /= Standard_Standard
         and then Handle_Use
@@ -7684,6 +7686,34 @@ package body Sem_Ch8 is
    -- Save_Scope_Stack --
    ----------------------
 
+   --  Save_Scope_Stack/Restore_Scope_Stack were originally designed to avoid
+   --  consuming any memory. That is, Save_Scope_Stack took care of removing
+   --  from immediate visibility entities and Restore_Scope_Stack took care
+   --  of restoring their visibility analyzing the context of each entity. The
+   --  problem of such approach is that it was fragile and caused unexpected
+   --  visibility problems, and indeed one test was found where there was a
+   --  real problem.
+
+   --  Furthermore, the following experiment was carried out:
+
+   --    - Save_Scope_Stack was modified to store in an Elist1 all those
+   --      entities whose attribute Is_Immediately_Visible is modified
+   --      from True to False.
+
+   --    - Restore_Scope_Stack was modified to store in another Elist2
+   --      all the entities whose attribute Is_Immediately_Visible is
+   --      modified from False to True.
+
+   --    - Extra code was added to verify that all the elements of Elist1
+   --      are found in Elist2
+
+   --  This test show that there may be more occurrences of this problem which
+   --  have not yet been detected. As a result, we replaced that approach by
+   --  the current one in which Save_Scope_Stack returns the list of entities
+   --  whose visibility is changed, and that list is passed to Restore_Scope
+   --  Stack to undo that change. This approach is simpler and safer, although
+   --  it consumes more memory.
+
    function Save_Scope_Stack (Handle_Use : Boolean := True) return Elist_Id is
       Result  : constant Elist_Id := New_Elmt_List;
       E       : Entity_Id;
@@ -7698,8 +7728,6 @@ package body Sem_Ch8 is
       -- Remove_From_Visibility --
       ----------------------------
 
-      --  Need comment on why we do this instead of old approach???
-
       procedure Remove_From_Visibility (E : Entity_Id) is
       begin
          if Is_Immediately_Visible (E) then
diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb
index c914703..623d3c4 100644
--- a/gcc/ada/sem_util.adb
+++ b/gcc/ada/sem_util.adb
@@ -14144,9 +14144,8 @@ package body Sem_Util is
       if Known_Static_Esize (Typ) then
          Size := UI_To_Int (Esize (Typ));
 
-      --  If the Esize (Object_Size) is unknown at compile-time, look at the
-      --  RM_Size (Value_Size) since it may have been set by an explicit rep
-      --  item.
+      --  If the Esize (Object_Size) is unknown at compile time, look at the
+      --  RM_Size (Value_Size) which may have been set by an explicit rep item.
 
       elsif Known_Static_RM_Size (Typ) then
          Size := UI_To_Int (RM_Size (Typ));