Patchwork [Ada] Checks fail on right operand of "and" and "or" with Short_Circuit_And_Or

login
register
mail settings
Submitter Arnaud Charlet
Date Oct. 14, 2011, 2:57 p.m.
Message ID <20111014145719.GA10373@adacore.com>
Download mbox | patch
Permalink /patch/119816/
State New
Headers show

Comments

Arnaud Charlet - Oct. 14, 2011, 2:57 p.m.
When the pragma Short_Circuit_And_Or is used, no part of the right operand
of an "and" or "or" operator should be executed if the left operand would
short-circuit the evaluation of the corresponding "and then" or "or else".
However, run-time checks associated with such operands were being evaluated
unconditionally, due to being before to the condition prior to the rewriting
as short-circuit forms during expansion. This is corrected by doing the
rewrite during analysis of the logical operators rather than waiting until
expansion.

Tested on x86_64-pc-linux-gnu, committed on trunk

2011-10-14  Gary Dismukes  <dismukes@adacore.com>

	* exp_ch4.adb (Expand_N_Op_And): Remove Short_Circuit_And_Or
	expansion code (moved to sem_res) (Expand_N_Op_Or): Remove
	Short_Circuit_And_Or expansion code (moved to sem_res).
	* sem_res.adb (Resolve_Logical_Op): Add code to rewrite Boolean
	"and" and "or" operators as short-circuit "and then" and "or
	else", when pragma Short_Circuit_And_Or is active.

Patch

Index: sem_res.adb
===================================================================
--- sem_res.adb	(revision 179984)
+++ sem_res.adb	(working copy)
@@ -7356,6 +7356,48 @@ 
          Check_For_Visible_Operator (N, B_Typ);
       end if;
 
+      --  Replace AND by AND THEN, or OR by OR ELSE, if Short_Circuit_And_Or
+      --  is active and the result type is standard Boolean (do not mess with
+      --  ops that return a nonstandard Boolean type, because something strange
+      --  is going on).
+
+      --  Note: you might expect this replacement to be done during expansion,
+      --  but that doesn't work, because when the pragma Short_Circuit_And_Or
+      --  is used, no part of the right operand of an "and" or "or" operator
+      --  should be executed if the left operand would short-circuit the
+      --  evaluation of the corresponding "and then" or "or else". If we left
+      --  the replacement to expansion time, then run-time checks associated
+      --  with such operands would be evaluated unconditionally, due to being
+      --  before to the condition prior to the rewriting as short-circuit forms
+      --  during expansion.
+
+      if Short_Circuit_And_Or
+        and then B_Typ = Standard_Boolean
+        and then Nkind_In (N, N_Op_And, N_Op_Or)
+      then
+         if Nkind (N) = N_Op_And then
+            Rewrite (N,
+              Make_And_Then (Sloc (N),
+                Left_Opnd  => Relocate_Node (Left_Opnd (N)),
+                Right_Opnd => Relocate_Node (Right_Opnd (N))));
+            Analyze_And_Resolve (N, B_Typ);
+
+         --  Case of OR changed to OR ELSE
+
+         else
+            Rewrite (N,
+              Make_Or_Else (Sloc (N),
+                Left_Opnd  => Relocate_Node (Left_Opnd (N)),
+                Right_Opnd => Relocate_Node (Right_Opnd (N))));
+            Analyze_And_Resolve (N, B_Typ);
+         end if;
+
+         --  Return now, since analysis of the rewritten ops will take care of
+         --  other reference bookkeeping and expression folding.
+
+         return;
+      end if;
+
       Resolve (Left_Opnd (N), B_Typ);
       Resolve (Right_Opnd (N), B_Typ);
 
Index: exp_ch4.adb
===================================================================
--- exp_ch4.adb	(revision 179984)
+++ exp_ch4.adb	(working copy)
@@ -5579,27 +5579,11 @@ 
          Expand_Boolean_Operator (N);
 
       elsif Is_Boolean_Type (Etype (N)) then
+         Adjust_Condition (Left_Opnd (N));
+         Adjust_Condition (Right_Opnd (N));
+         Set_Etype (N, Standard_Boolean);
+         Adjust_Result_Type (N, Typ);
 
-         --  Replace AND by AND THEN if Short_Circuit_And_Or active and the
-         --  type is standard Boolean (do not mess with AND that uses a non-
-         --  standard Boolean type, because something strange is going on).
-
-         if Short_Circuit_And_Or and then Typ = Standard_Boolean then
-            Rewrite (N,
-              Make_And_Then (Sloc (N),
-                Left_Opnd  => Relocate_Node (Left_Opnd (N)),
-                Right_Opnd => Relocate_Node (Right_Opnd (N))));
-            Analyze_And_Resolve (N, Typ);
-
-         --  Otherwise, adjust conditions
-
-         else
-            Adjust_Condition (Left_Opnd (N));
-            Adjust_Condition (Right_Opnd (N));
-            Set_Etype (N, Standard_Boolean);
-            Adjust_Result_Type (N, Typ);
-         end if;
-
       elsif Is_Intrinsic_Subprogram (Entity (N)) then
          Expand_Intrinsic_Call (N, Entity (N));
 
@@ -7535,27 +7519,11 @@ 
          Expand_Boolean_Operator (N);
 
       elsif Is_Boolean_Type (Etype (N)) then
+         Adjust_Condition (Left_Opnd (N));
+         Adjust_Condition (Right_Opnd (N));
+         Set_Etype (N, Standard_Boolean);
+         Adjust_Result_Type (N, Typ);
 
-         --  Replace OR by OR ELSE if Short_Circuit_And_Or active and the type
-         --  is standard Boolean (do not mess with AND that uses a non-standard
-         --  Boolean type, because something strange is going on).
-
-         if Short_Circuit_And_Or and then Typ = Standard_Boolean then
-            Rewrite (N,
-              Make_Or_Else (Sloc (N),
-                Left_Opnd  => Relocate_Node (Left_Opnd (N)),
-                Right_Opnd => Relocate_Node (Right_Opnd (N))));
-            Analyze_And_Resolve (N, Typ);
-
-         --  Otherwise, adjust conditions
-
-         else
-            Adjust_Condition (Left_Opnd (N));
-            Adjust_Condition (Right_Opnd (N));
-            Set_Etype (N, Standard_Boolean);
-            Adjust_Result_Type (N, Typ);
-         end if;
-
       elsif Is_Intrinsic_Subprogram (Entity (N)) then
          Expand_Intrinsic_Call (N, Entity (N));