Re: [Ada] Expansion of Ada2012 predicate checks for type conversions
On 8/18/2011 5:33 AM, Arnaud Charlet wrote: 2011-08-05 Ed Schonberg * exp_ch4.adb (Expand_N_Type_Conversion): When expanding a predicate check, indicate that the copy of the original node does not come from source, to prevent an infinite recursion of the expansion. For ChangeLog entries we usually, and per the GNU Coding Conventions, do not provide the "Why?", just the "What?". Yes we know about this and we'll have unfortunately to disagree on this one: we very strongly believe at AdaCore and for GNAT development that mentioning the why is much more useful than just the what and insist on doing so in our changelogs, rather than having to refer to separate emails for understanding a change. Having such detailled changelogs is very useful in practice to maintain code and modify it, at least that's our experience. So in other words, we find this GNU Coding Conventions a bad practice, and insist on not following it, intentionally. to add to this a bit. We do agree that having the "why" only in the changelog and not in the code is a bad idea. Indeed, it is critical that the code contain full comments (often for instance, critical comments are what you are NOT doing and why, this is one respect in which code can never be self documenting). I fully understand the concern about people using revision histories as a substitute for proper code comments. We never let that happen in the GNAT case, part of our review process ensures that any missing comments in the source get fixed before an FSF checkin. Arno
Re: [Ada] Expansion of Ada2012 predicate checks for type conversions
> > 2011-08-05 Ed Schonberg > > > > * exp_ch4.adb (Expand_N_Type_Conversion): When expanding a > > predicate > > check, indicate that the copy of the original node does not come from > > source, to prevent an infinite recursion of the expansion. > > For ChangeLog entries we usually, and per the GNU Coding Conventions, > do not provide the "Why?", just the "What?". Yes we know about this and we'll have unfortunately to disagree on this one: we very strongly believe at AdaCore and for GNAT development that mentioning the why is much more useful than just the what and insist on doing so in our changelogs, rather than having to refer to separate emails for understanding a change. Having such detailled changelogs is very useful in practice to maintain code and modify it, at least that's our experience. So in other words, we find this GNU Coding Conventions a bad practice, and insist on not following it, intentionally. Arno
Re: [Ada] Expansion of Ada2012 predicate checks for type conversions
On Fri, 5 Aug 2011, Arnaud Charlet wrote: > 2011-08-05 Ed Schonberg > > * exp_ch4.adb (Expand_N_Type_Conversion): When expanding a predicate > check, indicate that the copy of the original node does not come from > source, to prevent an infinite recursion of the expansion. For ChangeLog entries we usually, and per the GNU Coding Conventions, do not provide the "Why?", just the "What?". Gerald
[Ada] Expansion of Ada2012 predicate checks for type conversions
This patch fixes an infinite recursion in the application of predicate checks to type conversions. The following must compile quietly: gcc -c -gnat12 ./why-conversions.ads --- with Why.Ids; use Why.Ids; package Why.Conversions is function "+" (Id : W_Unused_At_Start_OId) return W_Unused_At_Start_Valid_OId; private function "+" (Id : W_Unused_At_Start_OId) return W_Unused_At_Start_Valid_OId is (W_Unused_At_Start_Valid_OId (Id)); end Why.Conversions; --- package Why is pragma Pure; end Why; --- with Why.Unchecked_Ids; use Why.Unchecked_Ids; package Why.Ids is subtype W_Unused_At_Start_Valid_OId is W_Unused_At_Start_Unchecked_OId; type W_Unused_At_Start_OId is new W_Unused_At_Start_Valid_OId; end Why.Ids; --- with Why.Opaque_Ids; use Why.Opaque_Ids; package Why.Kind_Validity is -- Kind-validity checks function Unused_At_Start_OId_Kind_Valid (Id : W_Unused_At_Start_Opaque_OId) return Boolean; private function Unused_At_Start_OId_Kind_Valid (Id : W_Unused_At_Start_Opaque_OId) return Boolean is (Id /= 0); end Why.Kind_Validity; --- package Why.Opaque_Ids is subtype W_Unused_At_Start_Opaque_OId is Integer; end Why.Opaque_Ids; --- with Why.Opaque_Ids;use Why.Opaque_Ids; with Why.Kind_Validity; use Why.Kind_Validity; package Why.Unchecked_Ids is subtype W_Unused_At_Start_Unchecked_OId is W_Unused_At_Start_Opaque_OId with Predicate => (Unused_At_Start_OId_Kind_Valid (W_Unused_At_Start_Unchecked_OId)); end Why.Unchecked_Ids; Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-05 Ed Schonberg * exp_ch4.adb (Expand_N_Type_Conversion): When expanding a predicate check, indicate that the copy of the original node does not come from source, to prevent an infinite recursion of the expansion. Index: exp_ch4.adb === --- exp_ch4.adb (revision 177431) +++ exp_ch4.adb (working copy) @@ -9154,8 +9154,16 @@ and then Target_Type /= Operand_Type and then Comes_From_Source (N) then - Insert_Action (N, - Make_Predicate_Check (Target_Type, Duplicate_Subexpr (N))); + declare +New_Expr : constant Node_Id := Duplicate_Subexpr (N); + + begin +-- Avoid infinite recursion on the subsequent expansion of +-- of the copy of the original type conversion. + +Set_Comes_From_Source (New_Expr, False); +Insert_Action (N, Make_Predicate_Check (Target_Type, New_Expr)); + end; end if; end Expand_N_Type_Conversion;
