Re: [PATCH 1/2 v3] Ada: Synchronized private extensions are always limited
Hi Marc, Indeed I have read and accept the terms of the DCO. Please proceed to apply these changes at your convenience. Thanks for helping me get these committed! Richard Wai > On Sep 18, 2023, at 04:31, Marc Poulhiès wrote: > > > Hello Richard, > >> I have added the required “Signed-off-by” tag to the patch and to the change >> log >> entry below. I believe for all other aspects I have followed the >> instructions. > > Thanks for doing these modifications. I believe you have read the > Developer's Certificate of Origin (https://gcc.gnu.org/dco.html) and > accept it? > >> For getting the patch applied it states "If you do not have write access and >> a >> patch of yours has been approved, but not committed, please advise the >> approver >> of that fact.” So I think I have done that correctly.. However let me know if >> there is someone else not included in the CC that should be handling that. > > I can take it from here and apply both your changes if that's ok with > you. > > Thanks for your patches! > > Marc
Re: [PATCH 2/2 v3] Ada: Finalization of constrained subtypes of unconstrained synchronized private extensions
Thanks for that! Richard > On Sep 18, 2023, at 09:43, Arnaud Charlet wrote: > >> Thanks for finding that! I have made the recommended change and attached the >> revised patch, which is also rebased on trunk. >> >> Additionally, I have added the “Signed-off-by” tag for legal compliance to >> the patch, as well as the change log entry as follows: > > Thank you, patch is therefore now OK.
[PATCH 1/2 v3] Ada: Synchronized private extensions are always limited
Hi Arno, I have added the required “Signed-off-by” tag to the patch and to the change log entry below. I believe for all other aspects I have followed the instructions. For getting the patch applied it states "If you do not have write access and a patch of yours has been approved, but not committed, please advise the approver of that fact.” So I think I have done that correctly.. However let me know if there is someone else not included in the CC that should be handling that. Of course, I’d love to work towards one day getting write access myself, but something tells me that’s a bit of a process. -- Begin change log entry -- ada: Private extensions with the keyword “synchronized” are always limited. GNAT was relying on synchronized private type extensions deriving from a concurrent interface to determine its limitedness. This does not cover the case where such an extension derives a limited interface. RM-7.6(6/2) makes is clear that “synchronized” in a private extension implies the derived type is limited. GNAT should explicitly check for the presence of “synchronized” in a private extension declaration, and it should have the same effect as the presence of “limited”. gcc/ada/ * sem_ch3.adb (Build_Derived_Record_Type): Treat presence of keyword “synchronized” the same as “limited” when determining if a private extension is limited. Signed-off-by: Richard Wai mailto:rich...@annexi-strayline.com>> -- End change log entry -- Thanks, Richard  > On Sep 13, 2023, at 03:54, Arnaud Charlet wrote: > >> No worries, and sorry for the trouble. I’m going to try using a different >> client for the gcc mailing list, it doesn’t seem to like Outlook. Thanks for >> catching that mistake! >> >> Please advise how I can get this patch actually applied, given my lack of >> commit privilege. > > You first need to follow instructions from https://gcc.gnu.org/contribute.html > and in particular meet the legal requirements. > > Then get someone with write approval to commit the change. > > Arno
Re: [PATCH 2/2 v3] Ada: Finalization of constrained subtypes of unconstrained synchronized private extensions
Hi Gary, Thanks for finding that! I have made the recommended change and attached the revised patch, which is also rebased on trunk. Additionally, I have added the “Signed-off-by” tag for legal compliance to the patch, as well as the change log entry as follows: -- Begin change log entry – ada: TSS finalize address subprogram generation for constrained subtypes of unconstrained synchronized private extensions should take care to designate the corresponding record of the underlying concurrent type. When generating TSS finalize address subprograms for class-wide types of constrained root types, it follows the parent chain looking for the first “non-constrained” type. It is possible that such a type is a private extension with the “synchronized” keyword, in which case the underlying type is a concurrent type. When that happens, the designated type of the finalize address subprogram should be the corresponding record’s class-wide-type. Gcc/ada/ * exp_ch3(Expand_Freeze_Class_Wide_Type): Expanded comments explaining why TSS Finalize_Address is not generated for concurrent class-wide types. * exp_ch7(Make_Finalize_Address_Stmts): Handle cases where the underlying non-constrained parent type is a concurrent type, and adjust the designated type to be the corresponding record’s class-wide type. Signed-off-by: Richard Wai mailto:rich...@annexi-strayline.com>> -- End change log entry – See you at the next meeting! Cheers, Richard  > On Sep 15, 2023, at 12:38, Gary Dismukes wrote: > > Richard, > > As a follow-on to my earlier message, additional testing has uncovered an > issue > with your patch. When run against a compiler built with assertions enabled, > the test of "Present (Corresponding_Record_Type (Parent_Utyp))" can fail. > An additional guard is needed prior to that test, as follows: > >if Ekind (Parent_Utyp) in Concurrent_Kind > and then Present (Corresponding_Record_Type (Parent_Utyp)) >then > Parent_Utyp := Corresponding_Record_Type (Parent_Utyp); >end if; > > -- Gary >
[PING][PATCH 2/2 v2] Ada: Finalization of constrained subtypes of unconstrained synchronized private extensions
> On Aug 23, 2023, at 10:24, Richard Wai wrote: > > Somehow an error worked its way into the original diff (the diff itself), > making the previous patch fail to apply. > > Fixed version attached. > > Richard Wai > ANNEXI-STRAYLINE > > From: Richard Wai <mailto:rich...@annexi-strayline.com>> > Sent: Thursday, August 10, 2023 1:27 AM > To: 'gcc-patches@gcc.gnu.org <mailto:gcc-patches@gcc.gnu.org>' > mailto:gcc-patches@gcc.gnu.org>> > Cc: 'Eric Botcazou' mailto:ebotca...@adacore.com>>; > 'Arnaud Charlet' mailto:char...@adacore.com>>; 'Stephen > Baird' mailto:ba...@adacore.com>> > Subject: [PATCH 2/2] Ada: Finalization of constrained subtypes of > unconstrained synchronized private extensions > > When generating TSS address finalization bodies for a tagged class-wide > subtype, GNAT climbs the parent chain looking for the first “non-constrained” > type. That type’s underlying type’s class-wide type is used as a “designated” > type for a dispatching TSS deep finalize call to the designated class-wide > type. In the case of a constrained subtype of an unconstrained synchronized > private extension, this ends up designating the underlying type of that > private extension. This means it targets the class-wide type of the actual > underlying concurrent type rather than the corresponding record. Ultimately > it ends up generating a call to the corresponding record’s deep finalizer, > but with incompatible types (concurrent_type’Class -> > concurrent_typeV’Class). This causes compilation to fail. > > This patch adds extra logic to exp_ch7(Make_Finalize_Address_Stmts) to > identify such cases and ensure that the designated type is the corresponding > record type’s class-wide type in that situation. > > Patch file is attached. > > -- Begin change log entry – > > ada: TSS finalize address subprogram generation for constrained subtypes of > unconstrained synchronized private extensions should take care to designate > the corresponding record of the underlying concurrent type. > > When generating TSS finalize address subprograms for class-wide types of > constrained root types, it follows the parent chain looking for the first > “non-constrained” type. It is possible that such a type is a private > extension with the “synchronized” keyword, in which case the underlying type > is a concurrent type. When that happens, the designated type of the finalize > address subprogram should be the corresponding record’s class-wide-type. > > Gcc/ada/ > * exp_ch3(Expand_Freeze_Class_Wide_Type): Expanded comments > explaining why TSS Finalize_Address is not generated for concurrent > class-wide types. > * exp_ch7(Make_Finalize_Address_Stmts): Handle cases where > the underlying non-constrained parent type is a concurrent type, and adjust > the designated type to be the corresponding record’s class-wide type. > > -- End change log entry – > > This patch was bootstrapped on x86_64-*-freebsd13.2. One new test cases was > added. Note that 4 gnat test cases fail currently on master and are unrelated > to this patch. > > Check-ada output of this patch: > > === acats tests === > Running chapter a ... > Running chapter c2 ... > Running chapter c3 ... > Running chapter c4 ... > Running chapter c5 ... > Running chapter c6 ... > Running chapter c7 ... > Running chapter c8 ... > Running chapter c9 ... > Running chapter ca ... > Running chapter cb ... > Running chapter cc ... > Running chapter cd ... > Running chapter ce ... > Running chapter cxa ... > Running chapter cxb ... > Running chapter cxf ... > Running chapter cxg ... > Running chapter cxh ... > Running chapter cz ... > Running chapter d ... > Running chapter e ... > Running chapter l ... > === acats Summary === > # of expected passes 2328 > # of unexpected failures 0 > > Native configuration is x86_64-unknown-freebsd13.2 > > === gnat tests === > > Schedule of variations: > unix > > Running target unix > FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 14) > FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 20) > FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 38) > FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 42) > > === gnat Summary === > > # of expected passes 3401 > # of unexpected failures 4 > # of expected failures 23 > # of unsupported tests 10 > gnatmake version 14.0.0 20230809 (experimental) > > > Richard Wai > ANNEXI-STRAYLINE 
[PATCH 1/2 v2] Ada: Synchronized private extensions are always limited
Hi Arno, No worries, and sorry for the trouble. I’m going to try using a different client for the gcc mailing list, it doesn’t seem to like Outlook. Thanks for catching that mistake! Please advise how I can get this patch actually applied, given my lack of commit privilege. Revised patch attached! Thanks! ada-synchronized-private-types-are-limited-v2.patch Description: Binary data > On Sep 1, 2023, at 08:08, Arnaud Charlet wrote: > >> For some reason, your email is endeing up in a strange format, I almost >> missed the .patch file attached, making the review harder. > > Never mind, I was on vacation earlier this month and then busy with a seminar > last week, so I started looking at your ping email before the original email > which did contain the patch easily found, sorry for the noise! > > Arno
[PATCH 2/2 v2] Ada: Finalization of constrained subtypes of unconstrained synchronized private extensions
Somehow an error worked its way into the original diff (the diff itself), making the previous patch fail to apply. Fixed version attached. Richard Wai ANNEXI-STRAYLINE From: Richard Wai Sent: Thursday, August 10, 2023 1:27 AM To: 'gcc-patches@gcc.gnu.org' Cc: 'Eric Botcazou' ; 'Arnaud Charlet' ; 'Stephen Baird' Subject: [PATCH 2/2] Ada: Finalization of constrained subtypes of unconstrained synchronized private extensions When generating TSS address finalization bodies for a tagged class-wide subtype, GNAT climbs the parent chain looking for the first "non-constrained" type. That type's underlying type's class-wide type is used as a "designated" type for a dispatching TSS deep finalize call to the designated class-wide type. In the case of a constrained subtype of an unconstrained synchronized private extension, this ends up designating the underlying type of that private extension. This means it targets the class-wide type of the actual underlying concurrent type rather than the corresponding record. Ultimately it ends up generating a call to the corresponding record's deep finalizer, but with incompatible types (concurrent_type'Class -> concurrent_typeV'Class). This causes compilation to fail. This patch adds extra logic to exp_ch7(Make_Finalize_Address_Stmts) to identify such cases and ensure that the designated type is the corresponding record type's class-wide type in that situation. Patch file is attached. -- Begin change log entry - ada: TSS finalize address subprogram generation for constrained subtypes of unconstrained synchronized private extensions should take care to designate the corresponding record of the underlying concurrent type. When generating TSS finalize address subprograms for class-wide types of constrained root types, it follows the parent chain looking for the first "non-constrained" type. It is possible that such a type is a private extension with the "synchronized" keyword, in which case the underlying type is a concurrent type. When that happens, the designated type of the finalize address subprogram should be the corresponding record's class-wide-type. Gcc/ada/ * exp_ch3(Expand_Freeze_Class_Wide_Type): Expanded comments explaining why TSS Finalize_Address is not generated for concurrent class-wide types. * exp_ch7(Make_Finalize_Address_Stmts): Handle cases where the underlying non-constrained parent type is a concurrent type, and adjust the designated type to be the corresponding record's class-wide type. -- End change log entry - This patch was bootstrapped on x86_64-*-freebsd13.2. One new test cases was added. Note that 4 gnat test cases fail currently on master and are unrelated to this patch. Check-ada output of this patch: === acats tests === Running chapter a ... Running chapter c2 ... Running chapter c3 ... Running chapter c4 ... Running chapter c5 ... Running chapter c6 ... Running chapter c7 ... Running chapter c8 ... Running chapter c9 ... Running chapter ca ... Running chapter cb ... Running chapter cc ... Running chapter cd ... Running chapter ce ... Running chapter cxa ... Running chapter cxb ... Running chapter cxf ... Running chapter cxg ... Running chapter cxh ... Running chapter cz ... Running chapter d ... Running chapter e ... Running chapter l ... === acats Summary === # of expected passes 2328 # of unexpected failures 0 Native configuration is x86_64-unknown-freebsd13.2 === gnat tests === Schedule of variations: unix Running target unix FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 14) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 20) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 38) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 42) === gnat Summary === # of expected passes 3401 # of unexpected failures 4 # of expected failures 23 # of unsupported tests 10 gnatmake version 14.0.0 20230809 (experimental) Richard Wai ANNEXI-STRAYLINE ada-tss-constrained-subtype-of-private-synchronized-extention-v2.patch Description: Binary data
[PING][PATCH 1/2] Ada: Synchronized private extensions are always limited
From: Richard Wai Sent: Thursday, August 10, 2023 12:55 AM To: 'gcc-patches@gcc.gnu.org' Cc: 'Eric Botcazou' ; 'Arnaud Charlet' ; 'Stephen Baird' Subject: [PATCH 1/2] Ada: Synchronized private extensions are always limited GNAT currently considers a synchronized private extension that derives from an interface to be limited only when said interface is a concurrent interface. However it is of course legal for a synchronized private extension to derive from a limited interface. In this case GNAT fails to correctly determine that the private extension is limited. This causes two separate problems that makes discriminated types in such a case impossible: 1. GNAT inappropriately rejects compilation, claiming default discriminants on such a private extension are illegal. 2. GNAT fails to generate the expected discriminals for the unconstrained discriminanted case, leading to the corresponding discriminants of the "corresponding record" of the underlying concurrent type to have no identifiers, and thus compilation fails. Fairly simple fix. If "synchronized" appears in the private extension declaration, it is limited. This is explicit in the RM as well (7.3(6/2)). Fixing this bug uncovered of a related bug wrt. TSS address finalizer generation for constrained subtypes of synchronized private extensions with no default discriminants. That patch is to follow separately. Patch file is attached. -- Begin change log entry -- ada: Private extensions with the keyword "synchronized" are always limited. GNAT was relying on synchronized private type extensions deriving from a concurrent interface to determine its limitedness. This does not cover the case where such an extension derives a limited interface. RM-7.6(6/2) makes is clear that "synchronized" in a private extension implies the derived type is limited. GNAT should explicitly check for the presence of "synchronized" in a private extension declaration, and it should have the same effect as the presence of "limited". gcc/ada/ * sem_ch3.adb (Build_Derived_Record_Type): Treat presence of keyword "synchronized" the same as "limited" when determining if a private extension is limited. -- End change log entry -- This patch was bootstrapped on x86_64-*-freebsd13.2. Two new test cases were added. Note that 4 gnat test cases fail currently on master and are unrelated to this patch. Check-ada output of this patch: === acats tests === Running chapter a ... Running chapter c2 ... Running chapter c3 ... Running chapter c4 ... Running chapter c5 ... Running chapter c6 ... Running chapter c7 ... Running chapter c8 ... Running chapter c9 ... Running chapter ca ... Running chapter cb ... Running chapter cc ... Running chapter cd ... Running chapter ce ... Running chapter cxa ... Running chapter cxb ... Running chapter cxf ... Running chapter cxg ... Running chapter cxh ... Running chapter cz ... Running chapter d ... Running chapter e ... Running chapter l ... === acats Summary === # of expected passes 2328 # of unexpected failures 0 Native configuration is x86_64-unknown-freebsd13.2 === gnat tests === Schedule of variations: unix Running target unix FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 14) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 20) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 38) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 42) === gnat Summary === # of expected passes 3402 # of unexpected failures4 # of expected failures 23 # of unsupported tests 10 gnatmake version 14.0.0 20230809 (experimental) Richard Wai ANNEXI-STRAYLINE
[PATCH 2/2] Ada: Finalization of constrained subtypes of unconstrained synchronized private extensions
When generating TSS address finalization bodies for a tagged class-wide subtype, GNAT climbs the parent chain looking for the first "non-constrained" type. That type's underlying type's class-wide type is used as a "designated" type for a dispatching TSS deep finalize call to the designated class-wide type. In the case of a constrained subtype of an unconstrained synchronized private extension, this ends up designating the underlying type of that private extension. This means it targets the class-wide type of the actual underlying concurrent type rather than the corresponding record. Ultimately it ends up generating a call to the corresponding record's deep finalizer, but with incompatible types (concurrent_type'Class -> concurrent_typeV'Class). This causes compilation to fail. This patch adds extra logic to exp_ch7(Make_Finalize_Address_Stmts) to identify such cases and ensure that the designated type is the corresponding record type's class-wide type in that situation. Patch file is attached. -- Begin change log entry - ada: TSS finalize address subprogram generation for constrained subtypes of unconstrained synchronized private extensions should take care to designate the corresponding record of the underlying concurrent type. When generating TSS finalize address subprograms for class-wide types of constrained root types, it follows the parent chain looking for the first "non-constrained" type. It is possible that such a type is a private extension with the "synchronized" keyword, in which case the underlying type is a concurrent type. When that happens, the designated type of the finalize address subprogram should be the corresponding record's class-wide-type. Gcc/ada/ * exp_ch3(Expand_Freeze_Class_Wide_Type): Expanded comments explaining why TSS Finalize_Address is not generated for concurrent class-wide types. * exp_ch7(Make_Finalize_Address_Stmts): Handle cases where the underlying non-constrained parent type is a concurrent type, and adjust the designated type to be the corresponding record's class-wide type. -- End change log entry - This patch was bootstrapped on x86_64-*-freebsd13.2. One new test cases was added. Note that 4 gnat test cases fail currently on master and are unrelated to this patch. Check-ada output of this patch: === acats tests === Running chapter a ... Running chapter c2 ... Running chapter c3 ... Running chapter c4 ... Running chapter c5 ... Running chapter c6 ... Running chapter c7 ... Running chapter c8 ... Running chapter c9 ... Running chapter ca ... Running chapter cb ... Running chapter cc ... Running chapter cd ... Running chapter ce ... Running chapter cxa ... Running chapter cxb ... Running chapter cxf ... Running chapter cxg ... Running chapter cxh ... Running chapter cz ... Running chapter d ... Running chapter e ... Running chapter l ... === acats Summary === # of expected passes 2328 # of unexpected failures 0 Native configuration is x86_64-unknown-freebsd13.2 === gnat tests === Schedule of variations: unix Running target unix FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 14) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 20) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 38) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 42) === gnat Summary === # of expected passes 3401 # of unexpected failures 4 # of expected failures 23 # of unsupported tests 10 gnatmake version 14.0.0 20230809 (experimental) Richard Wai ANNEXI-STRAYLINE ada-tss-constrained-subtype-of-private-synchronized-extention.patch Description: Binary data
[PATCH 1/2] Ada: Synchronized private extensions are always limited
GNAT currently considers a synchronized private extension that derives from an interface to be limited only when said interface is a concurrent interface. However it is of course legal for a synchronized private extension to derive from a limited interface. In this case GNAT fails to correctly determine that the private extension is limited. This causes two separate problems that makes discriminated types in such a case impossible: 1. GNAT inappropriately rejects compilation, claiming default discriminants on such a private extension are illegal. 2. GNAT fails to generate the expected discriminals for the unconstrained discriminanted case, leading to the corresponding discriminants of the "corresponding record" of the underlying concurrent type to have no identifiers, and thus compilation fails. Fairly simple fix. If "synchronized" appears in the private extension declaration, it is limited. This is explicit in the RM as well (7.3(6/2)). Fixing this bug uncovered of a related bug wrt. TSS address finalizer generation for constrained subtypes of synchronized private extensions with no default discriminants. That patch is to follow separately. Patch file is attached. -- Begin change log entry -- ada: Private extensions with the keyword "synchronized" are always limited. GNAT was relying on synchronized private type extensions deriving from a concurrent interface to determine its limitedness. This does not cover the case where such an extension derives a limited interface. RM-7.6(6/2) makes is clear that "synchronized" in a private extension implies the derived type is limited. GNAT should explicitly check for the presence of "synchronized" in a private extension declaration, and it should have the same effect as the presence of "limited". gcc/ada/ * sem_ch3.adb (Build_Derived_Record_Type): Treat presence of keyword "synchronized" the same as "limited" when determining if a private extension is limited. -- End change log entry -- This patch was bootstrapped on x86_64-*-freebsd13.2. Two new test cases were added. Note that 4 gnat test cases fail currently on master and are unrelated to this patch. Check-ada output of this patch: === acats tests === Running chapter a ... Running chapter c2 ... Running chapter c3 ... Running chapter c4 ... Running chapter c5 ... Running chapter c6 ... Running chapter c7 ... Running chapter c8 ... Running chapter c9 ... Running chapter ca ... Running chapter cb ... Running chapter cc ... Running chapter cd ... Running chapter ce ... Running chapter cxa ... Running chapter cxb ... Running chapter cxf ... Running chapter cxg ... Running chapter cxh ... Running chapter cz ... Running chapter d ... Running chapter e ... Running chapter l ... === acats Summary === # of expected passes 2328 # of unexpected failures 0 Native configuration is x86_64-unknown-freebsd13.2 === gnat tests === Schedule of variations: unix Running target unix FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 14) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 20) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 38) FAIL: gnat.dg/specs/alignment2.ads (test for warnings, line 42) === gnat Summary === # of expected passes 3402 # of unexpected failures4 # of expected failures 23 # of unsupported tests 10 gnatmake version 14.0.0 20230809 (experimental) Richard Wai ANNEXI-STRAYLINE ada-synchronized-private-types-are-limited.patch Description: Binary data
RE: [PATCH] Ada: hashed container Cursor type predefined equality non-conformance
Pinging this.. > -Original Message- > From: Richard Wai > Sent: March 16, 2021 2:19 PM > To: 'gcc-patches@gcc.gnu.org' > Cc: 'Arnaud Charlet' ; 'Bob Duff' > > Subject: RE: [PATCH] Ada: hashed container Cursor type predefined equality > non-conformance > > Just a note that I do not have write access, so I will need someone who does > to commit this patch, if approved. > > Richard > > > -Original Message- > > From: Richard Wai > > Sent: March 11, 2021 9:13 AM > > To: 'Arnaud Charlet' > > Cc: 'gcc-patches@gcc.gnu.org' ; 'Bob Duff' > > > > Subject: RE: [PATCH] Ada: hashed container Cursor type predefined > > equality non-conformance > > > > Here is the amended commit log: > > > > -- > > > > Ada: Ensure correct predefined equality behavior for Cursor objects of > > hashed containers. > > > > 2021-03-11 Richard Wai > > > > gcc/ada/ > > * libgnat/a-cohase.ads (Cursor): Synchronize comments for the > Cursor > > type definition to be consistent with identical definitions in other > > container packages. Add additional comments regarding the > importance > > of > > maintaining the "Position" component for predefined equality. > > * libgnat/a-cohama.ads (Cursor): Likewise. > > * libgnat/a-cihama.ads (Cursor): Likewise. > > * libgnat/a-cohase.adb (Find, Insert): Ensure that Cursor objects > > always have their "Position" component set to ensure predefined > > equality works as required. > > * libgnat/a-cohama.adb (Find, Insert): Likewise. > > * libgnat/a-cihama.adb (Find, Insert): Likewise. > > > > gcc/testsuite/ > > * gnat.dg/containers2.adb: New test. > > > > -- > > > > Thanks! > > > > Richard > > > > > -Original Message- > > > From: Arnaud Charlet > > > Sent: March 10, 2021 11:27 AM > > > To: Richard Wai > > > Cc: gcc-patches@gcc.gnu.org; 'Bob Duff' ; Arnaud > > > Charlet > > > Subject: Re: [PATCH] Ada: hashed container Cursor type predefined > > > equality non-conformance > > > > > > > I'm not sure I correctly understand you here, but my > > > > interpretation is that I should no longer submit Changelog > > > > entries, rather just the patch, and then > > > > > > Right. > > > > > > > a commit message (a-la git), and then presumably the Changelong > > > > entries will be generated automatically. From what I can see, gcc' > > > > website does not talk about that, so I'm guessing this format > > > > based on what I see from git-log, generally. > > > > > > > > So assuming that, attached is the "correct" patch, and here is the > > > > commit > > > > message: > > > > > > > > --- > > > > > > > > Author: Richard Wai > > > > > > > > Ada: Ensure correct Cursor predefined equality behavior for hashed > > > > containers. > > > > > > > > -- > > > > > > > > And for the record, the change log entries I've come up with as > > > > per the previous email: > > > > > > And the commit log will look like: > > > > > > 2021-03-09 Richard Wai > > > > > > gcc/ada/ > > > * libgnat/... > > > > > > gcc/testsuite/ > > > * gnat.dg/containers2.adb: ... > > > > > > Your patch is OK with these changes, thanks.
RE: [PATCH] Ada: hashed container Cursor type predefined equality non-conformance
Just a note that I do not have write access, so I will need someone who does to commit this patch, if approved. Richard > -Original Message- > From: Richard Wai > Sent: March 11, 2021 9:13 AM > To: 'Arnaud Charlet' > Cc: 'gcc-patches@gcc.gnu.org' ; 'Bob Duff' > > Subject: RE: [PATCH] Ada: hashed container Cursor type predefined equality > non-conformance > > Here is the amended commit log: > > -- > > Ada: Ensure correct predefined equality behavior for Cursor objects of > hashed containers. > > 2021-03-11 Richard Wai > > gcc/ada/ > * libgnat/a-cohase.ads (Cursor): Synchronize comments for the > Cursor > type definition to be consistent with identical definitions in other > container packages. Add additional comments regarding the > importance of > maintaining the "Position" component for predefined equality. > * libgnat/a-cohama.ads (Cursor): Likewise. > * libgnat/a-cihama.ads (Cursor): Likewise. > * libgnat/a-cohase.adb (Find, Insert): Ensure that Cursor objects > always have their "Position" component set to ensure predefined > equality works as required. > * libgnat/a-cohama.adb (Find, Insert): Likewise. > * libgnat/a-cihama.adb (Find, Insert): Likewise. > > gcc/testsuite/ > * gnat.dg/containers2.adb: New test. > > -- > > Thanks! > > Richard > > > -Original Message- > > From: Arnaud Charlet > > Sent: March 10, 2021 11:27 AM > > To: Richard Wai > > Cc: gcc-patches@gcc.gnu.org; 'Bob Duff' ; Arnaud > > Charlet > > Subject: Re: [PATCH] Ada: hashed container Cursor type predefined > > equality non-conformance > > > > > I'm not sure I correctly understand you here, but my interpretation > > > is that I should no longer submit Changelog entries, rather just the > > > patch, and then > > > > Right. > > > > > a commit message (a-la git), and then presumably the Changelong > > > entries will be generated automatically. From what I can see, gcc' > > > website does not talk about that, so I'm guessing this format based > > > on what I see from git-log, generally. > > > > > > So assuming that, attached is the "correct" patch, and here is the > > > commit > > > message: > > > > > > --- > > > > > > Author: Richard Wai > > > > > > Ada: Ensure correct Cursor predefined equality behavior for hashed > > > containers. > > > > > > -- > > > > > > And for the record, the change log entries I've come up with as per > > > the previous email: > > > > And the commit log will look like: > > > > 2021-03-09 Richard Wai > > > > gcc/ada/ > > * libgnat/... > > > > gcc/testsuite/ > > * gnat.dg/containers2.adb: ... > > > > Your patch is OK with these changes, thanks.
RE: [PATCH] Ada: hashed container Cursor type predefined equality non-conformance
Here is the amended commit log: -- Ada: Ensure correct predefined equality behavior for Cursor objects of hashed containers. 2021-03-11 Richard Wai gcc/ada/ * libgnat/a-cohase.ads (Cursor): Synchronize comments for the Cursor type definition to be consistent with identical definitions in other container packages. Add additional comments regarding the importance of maintaining the "Position" component for predefined equality. * libgnat/a-cohama.ads (Cursor): Likewise. * libgnat/a-cihama.ads (Cursor): Likewise. * libgnat/a-cohase.adb (Find, Insert): Ensure that Cursor objects always have their "Position" component set to ensure predefined equality works as required. * libgnat/a-cohama.adb (Find, Insert): Likewise. * libgnat/a-cihama.adb (Find, Insert): Likewise. gcc/testsuite/ * gnat.dg/containers2.adb: New test. -- Thanks! Richard > -Original Message- > From: Arnaud Charlet > Sent: March 10, 2021 11:27 AM > To: Richard Wai > Cc: gcc-patches@gcc.gnu.org; 'Bob Duff' ; Arnaud > Charlet > Subject: Re: [PATCH] Ada: hashed container Cursor type predefined equality > non-conformance > > > I'm not sure I correctly understand you here, but my interpretation is > > that I should no longer submit Changelog entries, rather just the > > patch, and then > > Right. > > > a commit message (a-la git), and then presumably the Changelong > > entries will be generated automatically. From what I can see, gcc' > > website does not talk about that, so I'm guessing this format based on > > what I see from git-log, generally. > > > > So assuming that, attached is the "correct" patch, and here is the > > commit > > message: > > > > --- > > > > Author: Richard Wai > > > > Ada: Ensure correct Cursor predefined equality behavior for hashed > > containers. > > > > -- > > > > And for the record, the change log entries I've come up with as per > > the previous email: > > And the commit log will look like: > > 2021-03-09 Richard Wai > > gcc/ada/ > * libgnat/... > > gcc/testsuite/ > * gnat.dg/containers2.adb: ... > > Your patch is OK with these changes, thanks.
RE: [PATCH] Ada: hashed container Cursor type predefined equality non-conformance
> And the commit log will look like: > > 2021-03-09 Richard Wai > > gcc/ada/ > * libgnat/... > > gcc/testsuite/ > * gnat.dg/containers2.adb: ... > > Your patch is OK with these changes, thanks. Understood! Thanks for that. Richard
RE: [PATCH] Ada: hashed container Cursor type predefined equality non-conformance
> -Original Message-
> From: Arnaud Charlet
> Sent: March 10, 2021 10:23 AM
> To: Richard Wai
> Cc: gcc-patches@gcc.gnu.org; 'Bob Duff' ; Arnaud
> Charlet
> Subject: Re: [PATCH] Ada: hashed container Cursor type predefined equality
> non-conformance
>
> > I appologise for making a mess of this final step. I have leared the
> > errors of my ways.
>
>
> By the way, ChangeLog are automatically generated these days, so we're
> really talking about commit logs and diffs against ChangeLog (as you did
in
> your previous email) are no longer appropriate (and almost never apply
> anyway).
>
I'm not sure I correctly understand you here, but my interpretation is that
I should no longer submit Changelog entries, rather just the patch, and then
a commit message (a-la git), and then presumably the Changelong entries will
be generated automatically. From what I can see, gcc' website does not talk
about that, so I'm guessing this format based on what I see from git-log,
generally.
So assuming that, attached is the "correct" patch, and here is the commit
message:
---
Author: Richard Wai
Ada: Ensure correct Cursor predefined equality behavior for hashed
containers.
--
And for the record, the change log entries I've come up with as per the
previous email:
--
gcc/ada/Changelog:
2021-03-09 Richard Wai
* libgnat/a-cohase.ads (Cursor): Synchronize comments for the Cursor
type definition to be consistent with identical definitions in other
container packages. Add additional comments regarding the importance
of
maintaining the "Position" component for predefined equality.
* libgnat/a-cohama.ads (Cursor): Likewise.
* libgnat/a-cihama.ads (Cursor): Likewise.
* libgnat/a-cohase.adb (Find, Insert): Ensure that Cursor objects
always have their "Position" component set to ensure predefined
equality works as required.
* libgnat/a-cohama.adb (Find, Insert): Likewise.
* libgnat/a-cihama.adb (Find, Insert): Likewise.
gcc/testsuite/Changelog:
2021-03-09 Richard Wai
* gnat.dg/containers2.adb: New test.
--
I've learned a lot, and appreciate your time.
Richard
container_cursor_equality_20210304.patch
Description: Binary data
RE: [PATCH] Ada: hashed container Cursor type predefined equality non-conformance
Arno, I appologise for making a mess of this final step. I have leared the errors of my ways. Below are the change log entries for this patch: gcc/ada/Changelog: 2021-03-09 Richard Wai * libgnat/a-cohase.ads (Cursor): Synchronize comments for the Cursor type definition to be consistent with identical definitions in other container packages. Add additional comments regarding the importance of maintaining the "Position" component for predefined equality. * libgnat/a-cohama.ads (Cursor): Likewise. * libgnat/a-cihama.ads (Cursor): Likewise. * libgnat/a-cohase.adb (Find, Insert): Ensure that Cursor objects always have their "Position" component set to ensure predefined equality works as required. * libgnat/a-cohama.adb (Find, Insert): Likewise. * libgnat/a-cihama.adb (Find, Insert): Likewise. gcc/testsuite/Changelog: 2021-03-09 Richard Wai * gnat.dg/containers2.adb: New test. Thanks for this. Richard > -Original Message- > From: Arnaud Charlet > Sent: March 9, 2021 10:51 AM > To: Richard Wai > Cc: gcc-patches@gcc.gnu.org; 'Bob Duff' ; Arnaud > Charlet > Subject: Re: [PATCH] Ada: hashed container Cursor type predefined equality > non-conformance > > > Oops, I wrote "effected" when I meant "affected". Of course, despite > > reading it several times before sending, I only noticed it after sending. > > > > Edited version: > > > > --- > > > > * libgnat/a-cohase.ads, libgnat/a-cohase.adb, libgnat/a-cohama.ads, > > libgnat/a-cohama.adb, libgnat/a-cihama.ads, libgnat/a-cihama.adb: > > Ensure that Cursor objects always have their "Position" component set > > to ensure predefined equality works as required; synchronize comments > > for the Cursor type defintion across affected packages, and add > > comments to explain the importance of updating the Position component. > > * testsuite/gnat.dg/containers2.adb: New regression test to ensure > > that all hashed containers obey predefined equality requirements for > Cursor objects. > > OK. The commit log follows strict rules, and in particular you need to list the > functions (And types modified), e.g: > > * libgnat/a-cohase.ads, libgnat/a-cohase.adb, libgnat/a-cohama.ads, > libgnat/a-cohama.adb, libgnat/a-cihama.ads, libgnat/a-cihama.adb > (Cursor, > Insert, Find): ... > > See https://gcc.gnu.org/codingconventions.html#ChangeLogs for more > details. > > OK with the above commit log amended. > > Arno
RE: [PATCH v2] Ada: hashed container Cursor type predefined equality non-conformance
Please disregard this v2 patch. V1 is the correct patch. Change log entries to be added for V1, to follow. Richard > -Original Message- > From: Richard Wai > Sent: March 9, 2021 4:26 PM > To: 'Arnaud Charlet' > Cc: 'gcc-patches@gcc.gnu.org' ; 'Bob Duff' > > Subject: [PATCH v2] Ada: hashed container Cursor type predefined equality > non-conformance > > Thanks for the pointer! > > I think Eric was a little bit too generous last time and formatted the commit > log for me =P. > > I have added the appropriate entries to the Changelog's, and have included > those in the patch (attached), which is also updated to master/HEAD, > hopefully that is the most helpful approach. > > Cheers, > > Richard > > > -Original Message- > > From: Arnaud Charlet > > Sent: March 9, 2021 10:51 AM > > To: Richard Wai > > Cc: gcc-patches@gcc.gnu.org; 'Bob Duff' ; Arnaud > > Charlet > > Subject: Re: [PATCH] Ada: hashed container Cursor type predefined > > equality non-conformance > > > > > Oops, I wrote "effected" when I meant "affected". Of course, despite > > > reading it several times before sending, I only noticed it after sending. > > > > > > Edited version: > > > > > > --- > > > > > > * libgnat/a-cohase.ads, libgnat/a-cohase.adb, libgnat/a-cohama.ads, > > > libgnat/a-cohama.adb, libgnat/a-cihama.ads, libgnat/a-cihama.adb: > > > Ensure that Cursor objects always have their "Position" component > > > set to ensure predefined equality works as required; synchronize > > > comments for the Cursor type defintion across affected packages, and > > > add comments to explain the importance of updating the Position > component. > > > * testsuite/gnat.dg/containers2.adb: New regression test to ensure > > > that all hashed containers obey predefined equality requirements for > > Cursor objects. > > > > OK. The commit log follows strict rules, and in particular you need to > > list the functions (And types modified), e.g: > > > > * libgnat/a-cohase.ads, libgnat/a-cohase.adb, libgnat/a-cohama.ads, > > libgnat/a-cohama.adb, libgnat/a-cihama.ads, libgnat/a-cihama.adb > > (Cursor, > > Insert, Find): ... > > > > See https://gcc.gnu.org/codingconventions.html#ChangeLogs for more > > details. > > > > OK with the above commit log amended. > > > > Arno
[PATCH v2] Ada: hashed container Cursor type predefined equality non-conformance
Thanks for the pointer! I think Eric was a little bit too generous last time and formatted the commit log for me =P. I have added the appropriate entries to the Changelog's, and have included those in the patch (attached), which is also updated to master/HEAD, hopefully that is the most helpful approach. Cheers, Richard > -Original Message- > From: Arnaud Charlet > Sent: March 9, 2021 10:51 AM > To: Richard Wai > Cc: gcc-patches@gcc.gnu.org; 'Bob Duff' ; Arnaud > Charlet > Subject: Re: [PATCH] Ada: hashed container Cursor type predefined equality > non-conformance > > > Oops, I wrote "effected" when I meant "affected". Of course, despite > > reading it several times before sending, I only noticed it after sending. > > > > Edited version: > > > > --- > > > > * libgnat/a-cohase.ads, libgnat/a-cohase.adb, libgnat/a-cohama.ads, > > libgnat/a-cohama.adb, libgnat/a-cihama.ads, libgnat/a-cihama.adb: > > Ensure that Cursor objects always have their "Position" component set > > to ensure predefined equality works as required; synchronize comments > > for the Cursor type defintion across affected packages, and add > > comments to explain the importance of updating the Position component. > > * testsuite/gnat.dg/containers2.adb: New regression test to ensure > > that all hashed containers obey predefined equality requirements for > Cursor objects. > > OK. The commit log follows strict rules, and in particular you need to list the > functions (And types modified), e.g: > > * libgnat/a-cohase.ads, libgnat/a-cohase.adb, libgnat/a-cohama.ads, > libgnat/a-cohama.adb, libgnat/a-cihama.ads, libgnat/a-cihama.adb > (Cursor, > Insert, Find): ... > > See https://gcc.gnu.org/codingconventions.html#ChangeLogs for more > details. > > OK with the above commit log amended. > > Arno container_cursor_equality_20210309.patch Description: Binary data
RE: [PATCH] Ada: hashed container Cursor type predefined equality non-conformance
Oops, I wrote "effected" when I meant "affected". Of course, despite reading it several times before sending, I only noticed it after sending. Edited version: --- * libgnat/a-cohase.ads, libgnat/a-cohase.adb, libgnat/a-cohama.ads, libgnat/a-cohama.adb, libgnat/a-cihama.ads, libgnat/a-cihama.adb: Ensure that Cursor objects always have their "Position" component set to ensure predefined equality works as required; synchronize comments for the Cursor type defintion across affected packages, and add comments to explain the importance of updating the Position component. * testsuite/gnat.dg/containers2.adb: New regression test to ensure that all hashed containers obey predefined equality requirements for Cursor objects. --- > -----Original Message- > From: Richard Wai > Sent: March 9, 2021 10:35 AM > To: 'Arnaud Charlet' > Cc: 'gcc-patches@gcc.gnu.org' ; 'Bob Duff' > > Subject: RE: [PATCH] Ada: hashed container Cursor type predefined equality > non-conformance > > Hey Arno, > > I forgot that last time as well! How about this: > > --- > > * libgnat/a-cohase.ads, libgnat/a-cohase.adb, libgnat/a-cohama.ads, > libgnat/a-cohama.adb, libgnat/a-cihama.ads, libgnat/a-cihama.adb: > Ensure that Cursor objects always have their "Position" component set to > ensure predefined equality works as required; synchronize comments for > the Cursor type defintion across effected packages, and add comments to > explain the importance of updating the Position component. > * testsuite/gnat.dg/containers2.adb: New regression test to ensure that all > hashed containers obey predefined equality requirements for Cursor > objects. > > --- > > Thanks for your speedy review! > > Richard > > > -Original Message- > > From: Arnaud Charlet > > Sent: March 9, 2021 3:12 AM > > To: Richard Wai > > Cc: gcc-patches@gcc.gnu.org; Arnaud Charlet ; Bob > > Duff > > Subject: Re: [PATCH] Ada: hashed container Cursor type predefined > > equality non-conformance > > > > Richard, > > > > Your patch is missing a corresponding commit log. > > > > Can you please add it to your submission? Otherwise your test and > > patch look good so far, thanks! > > > > Arno > > > > > We discovered an issue with the GNAT implementation of the hashed > > > container types.
RE: [PATCH] Ada: hashed container Cursor type predefined equality non-conformance
Hey Arno, I forgot that last time as well! How about this: --- * libgnat/a-cohase.ads, libgnat/a-cohase.adb, libgnat/a-cohama.ads, libgnat/a-cohama.adb, libgnat/a-cihama.ads, libgnat/a-cihama.adb: Ensure that Cursor objects always have their "Position" component set to ensure predefined equality works as required; synchronize comments for the Cursor type defintion across effected packages, and add comments to explain the importance of updating the Position component. * testsuite/gnat.dg/containers2.adb: New regression test to ensure that all hashed containers obey predefined equality requirements for Cursor objects. --- Thanks for your speedy review! Richard > -Original Message- > From: Arnaud Charlet > Sent: March 9, 2021 3:12 AM > To: Richard Wai > Cc: gcc-patches@gcc.gnu.org; Arnaud Charlet ; Bob > Duff > Subject: Re: [PATCH] Ada: hashed container Cursor type predefined equality > non-conformance > > Richard, > > Your patch is missing a corresponding commit log. > > Can you please add it to your submission? Otherwise your test and patch > look good so far, thanks! > > Arno > > > We discovered an issue with the GNAT implementation of the hashed > > container types.
[PATCH] Ada: hashed container Cursor type predefined equality non-conformance
Hi, We discovered an issue with the GNAT implementation of the hashed container types. The RM states (A.18-4-18/2, A.18.7-17/2, et al) that "the predefined "=" operator for type Cursor returns True if both cursors are No_Element, or designate the same element in the same container." In some cases, GNAT's implementation violates this requirement. This was due to the component "Position" of the Cursor type in Hashed_Sets, Hashed_Maps, and Indefinite_Hashed_Maps (though interestingly not in Indefinite_Hashed_Sets). The Position component is used to store the position of a node in a bucket, and is used internally as an optimization. Since it was viewed as an optimization, it was only updated opportunistically. However, this effects the predefined equality for the type. The result was that various Cursor objects could be returned which designated the same element in the same container, but yet evaluated as inequal. The attached patch ensures that the Position value is always updated when a Cursor object is returned or modified. It also synchronizes comments for the Cursor type definition across the various packages. Additionally, a new regression test case is added that checks for this issue among all four of the hashed container packages. This was successfully bootstrapped and tested on trunk, x86_64-unknown-freebsd12.2. Cheers, Richard Wai ANNEXI-STRAYLINE container_cursor_equality_20210304.patch Description: Binary data
RE: [PATCHv2] Ada: gcc-interface: fixed assertion for aliased entities
> -Original Message- > From: Eric Botcazou > Sent: March 11, 2020 6:16 AM > To: Richard Wai > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCHv2] Ada: gcc-interface: fixed assertion for aliased entities > > > Thanks for the analysis and the fix. This has indeed apparently never worked > and could be the origin of a few Ada PRs in Bugzilla, although I didn't really > check that. The patch is obviously very safe so I have installed it on the > mainline and the 9 branch (after slightly trimming down the comment) with > the following ChangeLogs (I forgot to say that we require them too): > > 2020-03-11 Richard Wai > > * gcc-interface/decl.c (gnat_to_gnu_entity): Also test Is_Public on > the Alias of the entitiy, if it is present, in the main assertion. > > 2020-03-11 Richard Wai > > * gnat.dg/subpools1.adb: New test. > > > [The next time please send the patch and the testcase as attachments > instead, they apparently have been mangled by your mailer.] > Thanks for this; all points noted! Richard Wai ANNEXI-STRAYLINE
[PATCHv2] Ada: gcc-interface: fixed assertion for aliased entities
in not null Subpool_Handle) + is + type Storage_Array_Access is access Storage_Array; + + New_Alloc: Storage_Array_Access + := new Storage_Array (1 .. Size_In_Storage_Elements + Alignment); + begin + for SE of New_Alloc.all loop +Storage_Address := SE'Address; +exit when Storage_Address mod Alignment = 0; + end loop; + end Allocate_From_Subpool; + + overriding + function Default_Subpool_For_Pool (Pool: in out Local_Pool) +return not null Subpool_Handle +is (Dummy_Subpool'Unchecked_Access); + + end Local_Pools; + + + A_Pool: Local_Pools.Local_Pool; + A_Subpool: Subpool_Handle := A_Pool.Create_Subpool; + + type Integer_Access is access Integer with + Storage_Pool => A_Pool; + + X: Integer_Access := new Integer; + +begin + null; +end Subpools1; Bootstrapped and tested from trunk on x86_64-unknown-freebsd12.1. Cheers, Richard Wai ANNEXI-STRAYLINE > -Original Message- > From: Eric Botcazou > Sent: March 7, 2020 5:48 AM > To: Richard Wai > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] Ada: gcc-interface: fixed assertion for aliased entities > > > Please excuse my ignorance as this is my first (and hopefully not > > last) patch submission.. But I don't see any testcases in the Ada > > testsuite except for the (outdated) ACATS tests, which doesn't cover > > this assertion. So I'm honestly not sure how I should go about that.. > > See testsuite/gnat.dg, there are around 3000 regressions tests. > > > Shall I add the Is_Public check to the Alias and resubmit the patch? > > Yes, please, but it can only be approved with an associated testcase. > > -- > Eric Botcazou
RE: [PATCH] Ada: gcc-interface: fixed assertion for aliased entities
> -Original Message- > From: Eric Botcazou > Sent: March 4, 2020 6:18 AM > To: Richard Wai > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] Ada: gcc-interface: fixed assertion for aliased entities > > > I'll have to look into this.. Any pointers? This assertion is not a > > language rule assertion. > > Of course, neither of the 117 assertions in gcc-interface is, instead they are > assertions meant to prevent wrong-code generation from occuring. > Please excuse my ignorance as this is my first (and hopefully not last) patch submission.. But I don't see any testcases in the Ada testsuite except for the (outdated) ACATS tests, which doesn't cover this assertion. So I'm honestly not sure how I should go about that.. > > As you see, the assertion being modified already tests for > > "Is_Public". So the issue is precisely that this assertion wrongly > > fails for cases where the entity is not Public. > > We cannot let anything go through if there is not Is_Public set somewhere, > possibly on the Alias (gnat_entity) since it is present in your case. > My reasoning there was that at decl.c:3914 when the E_Function/E_Procedure that has alias is resolved, there is a recursive call to gnat_to_gnu_entity on the alias. So if the alias was not Public, the same assertion will be triggered on that recursive call, so there seems to be no need to check it at that point. Though I suppose that could leave some holes if the incoming entity is not a subprogram! Shall I add the Is_Public check to the Alias and resubmit the patch? Richard Wai ANNEXI-STRAYLINE
RE: [PATCH] Ada: gcc-interface: fixed assertion for aliased entities
Richard Wai Managing Director T. 416.316.9806 > -Original Message- > From: Eric Botcazou > Sent: March 3, 2020 3:50 PM > To: Richard Wai > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] Ada: gcc-interface: fixed assertion for aliased entities > > > Discovered this error when attempting to allocate from a > > Root_Storage_Pool_With_Subpools derived type. If the type is derived > > in the current compilation unit, and Allocate is not overridden on > > derivation (as is typically the case with > > Root_Storage_Pool_With_Subpools), the entity for Allocate for the > > derived type is then an alias to > > System.Storage_Pools.Subpools.Allocate. When the allocator is built, > > gnat_to_gnu_entity is called with definition == false for the derived > > storage pool's allocate operation. An assertion is gnat_to_gnu_entity > > fails in this case, since it is not a definition, and Is_Public is > > false. If the storage pool type was instead derived in a different > compilation unit, this assertion is not triggered since the aliased entity has > the Public property. > > We need a testcase here, we cannot relax assertions without testcases. > I'll have to look into this.. Any pointers? This assertion is not a language rule assertion. > > This patch adds an extra check in the assertion (decl.c: > > gnat_to_gnu_entity) that the entity has the Aliased property. Also > > included a comment that describes the special case as per the description > above. > > I don't really understand the new condition, did you forget to test Is_Public? > As you see, the assertion being modified already tests for "Is_Public". So the issue is precisely that this assertion wrongly fails for cases where the entity is not Public. The specific case we ran into is where you have a derived Root_Storage_Pool_With_Subpools type where Allocate (resp. Deallocate) is inherited. If that derived type is anywhere except a package specification, The N_Defining_Identifier for allocate for the derived subpool will both be an alias to System.Storage_Pools.Subpools.Allocate, and will NOT be Public. It will then cause this assertion to fail upon building of the allocator or deallocator. > -- > Eric Botcazou Richard
[PATCH] Ada: gcc-interface: fixed assertion for aliased entities
Hi, Discovered this error when attempting to allocate from a Root_Storage_Pool_With_Subpools derived type. If the type is derived in the current compilation unit, and Allocate is not overridden on derivation (as is typically the case with Root_Storage_Pool_With_Subpools), the entity for Allocate for the derived type is then an alias to System.Storage_Pools.Subpools.Allocate. When the allocator is built, gnat_to_gnu_entity is called with definition == false for the derived storage pool's allocate operation. An assertion is gnat_to_gnu_entity fails in this case, since it is not a definition, and Is_Public is false. If the storage pool type was instead derived in a different compilation unit, this assertion is not triggered since the aliased entity has the Public property. This patch adds an extra check in the assertion (decl.c: gnat_to_gnu_entity) that the entity has the Aliased property. Also included a comment that describes the special case as per the description above. Bootstrapped and tested on x86_64-unknown-freebsd12.1 with no regressions. diff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c index 871a309ab7d..5ea930f4f65 100644 --- a/gcc/ada/gcc-interface/decl.c +++ b/gcc/ada/gcc-interface/decl.c @@ -447,6 +447,15 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) /* If we get here, it means we have not yet done anything with this entity. If we are not defining it, it must be a type or an entity that is defined elsewhere or externally, otherwise we should have defined it already. */ + + /* One exception relates to an entity, typically an inherited operation, + which has an alias pointing to the parent's operation. Often such an + aliased entity will also carry with it the Is_Public property if it was + declared in a separate compilation unit, but when a type is extended + within the current unit, the aliased entity will not pass this + assertion. It is neither defined (since it is an inherited operation, + and is not Public, since it is within the current compilation unit. */ + gcc_assert (definition || is_type || kind == E_Discriminant @@ -454,6 +463,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition) || kind == E_Label || (kind == E_Constant && Present (Full_View (gnat_entity))) || Is_Public (gnat_entity) + || Present (Alias (gnat_entity)) || type_annotate_only); /* Get the name of the entity and set up the line number and filename of Thanks, Richard Wai ANNEXI-STRAYLINE
