As explained in the previous change, Remove_Side_Effects already knows to
avoid renaming function calls in most cases and, more generally, to avoid
renaming something that does not denote a name. But the implementation is
a little ad-hoc and not bullet-proof, for example a function call subject
to an unchecked conversion will be renamed.
The change introduces a new predicate Is_Name_Reference, which is closely
modeled on Is_Object_Reference but returns true only for nodes that denote
a name and arranges for Remove_Side_Effects to generate a renaming only in
this case.
No functional changes.
Tested on x86_64-pc-linux-gnu, committed on trunk
2015-05-26 Eric Botcazou <ebotca...@adacore.com>
* exp_ch6.adb (Add_Call_By_Copy_Code): Remove restrictive
condition in the detection of the effects of Remove_Side_Effects.
* exp_util.ads (Remove_Side_Effects): Add general and historical note.
* exp_util.adb (Is_Name_Reference): New predicate.
(Remove_Side_Effects): Use it in lieu of Is_Object_Reference
in order to decide whether to use the renaming to capture the
side effects of the subexpression.
(Side_Effect_Free): Remove obsolete test.
Index: exp_util.adb
===================================================================
--- exp_util.adb (revision 223667)
+++ exp_util.adb (working copy)
@@ -7428,6 +7428,12 @@
-- is present (xxx is taken from the Chars field of Related_Nod),
-- otherwise it generates an internal temporary.
+ function Is_Name_Reference (N : Node_Id) return Boolean;
+ -- Determine if the tree referenced by N represents a name. This is
+ -- similar to Is_Object_Reference but returns true only if N can be
+ -- renamed without the need for a temporary, the typical example of
+ -- an object not in this category being a function call.
+
---------------------
-- Build_Temporary --
---------------------
@@ -7458,6 +7464,58 @@
end if;
end Build_Temporary;
+ -----------------------
+ -- Is_Name_Reference --
+ -----------------------
+
+ function Is_Name_Reference (N : Node_Id) return Boolean is
+ begin
+ if Is_Entity_Name (N) then
+ return Present (Entity (N)) and then Is_Object (Entity (N));
+ end if;
+
+ case Nkind (N) is
+ when N_Indexed_Component | N_Slice =>
+ return
+ Is_Name_Reference (Prefix (N))
+ or else Is_Access_Type (Etype (Prefix (N)));
+
+ -- Attributes 'Input, 'Old and 'Result produce objects
+
+ when N_Attribute_Reference =>
+ return
+ Nam_In
+ (Attribute_Name (N), Name_Input, Name_Old, Name_Result);
+
+ when N_Selected_Component =>
+ return
+ Is_Name_Reference (Selector_Name (N))
+ and then
+ (Is_Name_Reference (Prefix (N))
+ or else Is_Access_Type (Etype (Prefix (N))));
+
+ when N_Explicit_Dereference =>
+ return True;
+
+ -- A view conversion of a tagged name is a name reference
+
+ when N_Type_Conversion =>
+ return Is_Tagged_Type (Etype (Subtype_Mark (N)))
+ and then Is_Tagged_Type (Etype (Expression (N)))
+ and then Is_Name_Reference (Expression (N));
+
+ -- An unchecked type conversion is considered to be a name if
+ -- the operand is a name (this construction arises only as a
+ -- result of expansion activities).
+
+ when N_Unchecked_Type_Conversion =>
+ return Is_Name_Reference (Expression (N));
+
+ when others =>
+ return False;
+ end case;
+ end Is_Name_Reference;
+
-- Local variables
Loc : constant Source_Ptr := Sloc (Exp);
@@ -7498,34 +7556,25 @@
return;
end if;
- -- The remaining procesaing is done with all checks suppressed
+ -- The remaining processing is done with all checks suppressed
-- Note: from now on, don't use return statements, instead do a goto
-- Leave, to ensure that we properly restore Scope_Suppress.Suppress.
Scope_Suppress.Suppress := (others => True);
- -- If it is a scalar type and we need to capture the value, just make
- -- a copy. Likewise for a function call, an attribute reference, a
- -- conditional expression, an allocator, or an operator. And if we have
- -- a volatile reference and Name_Req is not set (see comments for
- -- Side_Effect_Free).
+ -- If it is an elementary type and we need to capture the value, just
+ -- make a constant. Likewise if this is not a name reference, except
+ -- for a type conversion because we would enter an infinite recursion
+ -- with Checks.Apply_Predicate_Check if the target type has predicates.
+ -- And type conversions need a specific treatment anyway, see below.
+ -- Also do it if we have a volatile reference and Name_Req is not set
+ -- (see comments for Side_Effect_Free).
if Is_Elementary_Type (Exp_Type)
-
- -- Note: this test is rather mysterious??? Why can't we just test ONLY
- -- Is_Elementary_Type and be done with it. If we try that approach, we
- -- get some failures (infinite recursions) from the Duplicate_Subexpr
- -- call at the end of Checks.Apply_Predicate_Check. To be
- -- investigated ???
-
and then (Variable_Ref
- or else Nkind_In (Exp, N_Attribute_Reference,
- N_Allocator,
- N_Case_Expression,
- N_If_Expression,
- N_Function_Call)
- or else Nkind (Exp) in N_Op
+ or else (not Is_Name_Reference (Exp)
+ and then Nkind (Exp) /= N_Type_Conversion)
or else (not Name_Req
and then Is_Volatile_Reference (Exp)))
then
@@ -7645,21 +7694,14 @@
Insert_Action (Exp, E);
end if;
- -- For expressions that denote objects, we can use a renaming scheme.
+ -- For expressions that denote names, we can use a renaming scheme.
-- This is needed for correctness in the case of a volatile object of
-- a non-volatile type because the Make_Reference call of the "default"
-- approach would generate an illegal access value (an access value
-- cannot designate such an object - see Analyze_Reference).
- elsif Is_Object_Reference (Exp)
- and then Nkind (Exp) /= N_Function_Call
+ elsif Is_Name_Reference (Exp)
- -- In Ada 2012 a qualified expression is an object, but for purposes
- -- of removing side effects it still need to be transformed into a
- -- separate declaration, particularly in the case of an aggregate.
-
- and then Nkind (Exp) /= N_Qualified_Expression
-
-- We skip using this scheme if we have an object of a volatile
-- type and we do not have Name_Req set true (see comments for
-- Side_Effect_Free).
@@ -7667,38 +7709,14 @@
and then (Name_Req or else not Treat_As_Volatile (Exp_Type))
then
Def_Id := Build_Temporary (Loc, 'R', Exp);
+ Res := New_Occurrence_Of (Def_Id, Loc);
- if Nkind (Exp) = N_Selected_Component
- and then Nkind (Prefix (Exp)) = N_Function_Call
- and then Is_Array_Type (Exp_Type)
- then
- -- Avoid generating a variable-sized temporary, by generating
- -- the renaming declaration just for the function call. The
- -- transformation could be refined to apply only when the array
- -- component is constrained by a discriminant???
+ Insert_Action (Exp,
+ Make_Object_Renaming_Declaration (Loc,
+ Defining_Identifier => Def_Id,
+ Subtype_Mark => New_Occurrence_Of (Exp_Type, Loc),
+ Name => Relocate_Node (Exp)));
- Res :=
- Make_Selected_Component (Loc,
- Prefix => New_Occurrence_Of (Def_Id, Loc),
- Selector_Name => Selector_Name (Exp));
-
- Insert_Action (Exp,
- Make_Object_Renaming_Declaration (Loc,
- Defining_Identifier => Def_Id,
- Subtype_Mark =>
- New_Occurrence_Of (Base_Type (Etype (Prefix (Exp))), Loc),
- Name => Relocate_Node (Prefix (Exp))));
-
- else
- Res := New_Occurrence_Of (Def_Id, Loc);
-
- Insert_Action (Exp,
- Make_Object_Renaming_Declaration (Loc,
- Defining_Identifier => Def_Id,
- Subtype_Mark => New_Occurrence_Of (Exp_Type, Loc),
- Name => Relocate_Node (Exp)));
- end if;
-
-- If this is a packed reference, or a selected component with
-- a non-standard representation, a reference to the temporary
-- will be replaced by a copy of the original expression (see
@@ -7715,8 +7733,20 @@
Set_Is_Renaming_Of_Object (Def_Id, False);
end if;
- -- Otherwise we generate a reference to the value
+ -- Avoid generating a variable-sized temporary, by generating the
+ -- reference just for the function call. The transformation could be
+ -- refined to apply only when the array component is constrained by a
+ -- discriminant???
+ elsif Nkind (Exp) = N_Selected_Component
+ and then Nkind (Prefix (Exp)) = N_Function_Call
+ and then Is_Array_Type (Exp_Type)
+ then
+ Remove_Side_Effects (Prefix (Exp), Name_Req, Variable_Ref);
+ goto Leave;
+
+ -- Otherwise we generate a reference to the expression
+
else
-- An expression which is in SPARK mode is considered side effect
-- free if the resulting value is captured by a variable or a
@@ -8974,23 +9004,10 @@
return Side_Effect_Free (Expression (N), Name_Req, Variable_Ref);
-- A selected component is side effect free only if it is a side
- -- effect free prefixed reference. If it designates a component
- -- with a rep. clause it must be treated has having a potential
- -- side effect, because it may be modified through a renaming, and
- -- a subsequent use of the renaming as a macro will yield the
- -- wrong value. This complex interaction between renaming and
- -- removing side effects is a reminder that the latter has become
- -- a headache to maintain, and that it should be removed in favor
- -- of the gcc mechanism to capture values ???
+ -- effect free prefixed reference.
when N_Selected_Component =>
- if Nkind (Parent (N)) = N_Explicit_Dereference
- and then Has_Non_Standard_Rep (Designated_Type (Typ))
- then
- return False;
- else
- return Safe_Prefixed_Reference (N);
- end if;
+ return Safe_Prefixed_Reference (N);
-- A range is side effect free if the bounds are side effect free
Index: exp_util.ads
===================================================================
--- exp_util.ads (revision 223661)
+++ exp_util.ads (working copy)
@@ -872,8 +872,8 @@
-- call and is analyzed and resolved on return. Name_Req may only be set to
-- True if Exp has the form of a name, and the effect is to guarantee that
-- any replacement maintains the form of name. If Renaming_Req is set to
- -- TRUE, the routine produces an object renaming reclaration capturing the
- -- expression. If Variable_Ref is set to TRUE, a variable is considered as
+ -- True, the routine produces an object renaming reclaration capturing the
+ -- expression. If Variable_Ref is set to True, a variable is considered as
-- side effect (used in implementing Force_Evaluation). Note: after call to
-- Remove_Side_Effects, it is safe to call New_Copy_Tree to obtain a copy
-- of the resulting expression.
@@ -885,6 +885,26 @@
-- Chars (Related_Id)_FIRST/_LAST. If Related_Id is set, then exactly one
-- of the Is_xxx_Bound flags must be set. For use of these parameters see
-- the warning in the body of Sem_Ch3.Process_Range_Expr_In_Decl.
+ --
+ -- The side effects are captured using one of the following methods:
+ --
+ -- 1) a constant initialized with the value of the subexpression
+ -- 2) a renaming of the subexpression
+ -- 3) a reference to the subexpression
+ --
+ -- For elementary types, methods 1) and 2) are used; for composite types,
+ -- methods 2) and 3) are used. The renaming (method 2) is used only when
+ -- the subexpression denotes a name, so that it can be elaborated by gigi
+ -- without evaluating the subexpression.
+ --
+ -- Historical note: the reference (method 3) used to be the common fallback
+ -- method but it gives rise to aliasing issues if the subexpression denotes
+ -- a name that is not aliased, since it is equivalent to taking the address
+ -- in this case. The renaming (method 2) used to be applied to any objects
+ -- in the RM sense, that is to say to the cases where a renaming is legal
+ -- in Ada. But for some of these cases, most notably functions calls, the
+ -- renaming cannot be elaborated without evaluating the subexpression, so
+ -- gigi would resort to method 1) or 3) under the hood for them.
function Represented_As_Scalar (T : Entity_Id) return Boolean;
-- Returns True iff the implementation of this type in code generation
Index: exp_ch6.adb
===================================================================
--- exp_ch6.adb (revision 223667)
+++ exp_ch6.adb (working copy)
@@ -1257,7 +1257,6 @@
begin
if Is_Renaming_Of_Object (Var)
and then Nkind (Renamed_Object (Var)) = N_Selected_Component
- and then Is_Entity_Name (Prefix (Renamed_Object (Var)))
and then Nkind (Original_Node (Prefix (Renamed_Object (Var))))
= N_Indexed_Component
and then
