[PATCH] Add Myself to Write After Approval and DCO List
Hello, I have added myself to write after approval. Thanks, Alexander Westbrooks >From 564b307970d3be7a02e827420f0fad87bd335d9b Mon Sep 17 00:00:00 2001 From: Alexander Westbrooks Date: Sat, 10 Feb 2024 12:20:00 -0600 Subject: [PATCH] Add Myself to Write After Approval and DCO List ChangeLog: * MAINTAINERS: Add myself to write after approval and DCO. Signed-off-by: Alexander Westbrooks --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3720344308e..617133447f0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -714,6 +714,7 @@ Stephen M. Webb John Wehle Florian Weimer Zack Weinberg +Alexander Westbrooks Mark Wielaard Edmar Wienskoski Ollie Wild @@ -778,4 +779,5 @@ Edward Smith-Rowland Petter Tomner Martin Uecker Jonathan Wakely +Alexander Westbrooks Chung-Ju Wu -- 2.25.1
[PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268]
Hello,
I have implemented a patch that fixes compile time errors for valid PDT
type-bound procedures. I wrote 4 new tests that address the test-cases in
PR 82943, PR 86148, and PR 86268, since the patch fixes all three of them.
All regression tests pass, including the new ones. This was tested on WSL
2, with Ubuntu 20.04 distro.
Is this okay to push to the trunk?
Alexander Westbrooks
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>From 100508673ae26d7fa4ae4f976b4542e115fc7b45 Mon Sep 17 00:00:00 2001
From: Alexander Westbrooks
Date: Sat, 10 Feb 2024 13:19:08 -0600
Subject: [PATCH] Fortran - Error compiling PDT Type-bound Procedures
[PR82943/86148/86268]
This patch allows parameterized derived types to compile successfully
when typebound procedures are specified in the type specification.
Furthermore, it allows function calls for PDTs by setting the
f2k_derived space of PDT instances to reference their original template,
thereby giving it referential access to the typebound procedures of the
template.
2024-02-10 Alexander Westbrooks
gcc/fortran/ChangeLog:
PR fortran/82943
PR fortran/86148
PR fortran/86268
* decl.cc (gfc_get_pdt_instance): Set the PDT instance field
'f2k_derived', if not set already, to point to the given
PDT template 'f2k_derived' namespace in order to give the
PDT instance referential access to the typebound procedures
of the template.
* gfortran.h (gfc_pdt_is_instance_of): Add prototype.
* resolve.cc (resolve_typebound_procedure): If the derived type
does not have the attribute 'pdt_template' set, compare the
dummy argument to the 'resolve_bindings_derived' type like usual.
If the derived type is a 'pdt_template', then check if the
dummy argument is an instance of the PDT template. If the derived
type is a PDT template, and the dummy argument is an instance of
that template, but the dummy argument 'param_list' is not
SPEC_ASSUMED, check if there are any LEN parameters in the
dummy argument. If there are no LEN parameters, then this implies
that there are only KIND parameters in the dummy argument.
If there are LEN parameters, this would be an error, for all
LEN parameters for the dummy argument MUST be assumed for
typebound procedures of PDTs.
* symbol.cc (gfc_pdt_is_instance_of): New function.
gcc/testsuite/ChangeLog:
PR fortran/82943
PR fortran/86148
PR fortran/86268
* gfortran.dg/pdt_34.f03: New test.
* gfortran.dg/pdt_35.f03: New test.
* gfortran.dg/pdt_36.f03: New test.
* gfortran.dg/pdt_37.f03: New test.
Signed-off-by: Alexander Westbrooks
---
gcc/fortran/decl.cc | 15 ++
gcc/fortran/gfortran.h | 1 +
gcc/fortran/resolve.cc | 68
gcc/fortran/symbol.cc| 29
gcc/testsuite/gfortran.dg/pdt_34.f03 | 42 +
gcc/testsuite/gfortran.dg/pdt_35.f03 | 45 ++
gcc/testsuite/gfortran.dg/pdt_36.f03 | 65 ++
gcc/testsuite/gfortran.dg/pdt_37.f03 | 34 ++
8 files changed, 291 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/pdt_34.f03
create mode 100644 gcc/testsuite/gfortran.dg/pdt_35.f03
create mode 100644 gcc/testsuite/gfortran.dg/pdt_36.f03
create mode 100644 gcc/testsuite/gfortran.dg/pdt_37.f03
diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc
index 503ecb8d9b5..c29b2bb0f45 100644
--- a/gcc/fortran/decl.cc
+++ b/gcc/fortran/decl.cc
@@ -4083,6 +4083,21 @@ gfc_get_pdt_instance (gfc_actual_arglist
*param_list, gfc_symbol **sym,
continue;
}
+ /*
+Addressing PR82943, this will fix the issue where a
function/subroutine is declared as not
+a member of the PDT instance. The reason for this is because the PDT
instance did not have
+access to its template's f2k_derived namespace in order to find the
typebound procedures.
+
+The number of references to the PDT template's f2k_derived will ensure
that f2k_derived is
+properly freed later on.
+ */
+
+ if (!instance->f2k_derived && pdt->f2k_derived)
+ {
+instance->f2k_derived = pdt->f2k_derived;
+instance->f2k_derived->refs++;
+ }
+
/* Set the component kind using the parameterized expression. */
if ((c1->ts.kind == 0 || c1->ts.type == BT_CHARACTER)
&& c1->kind_expr != NULL)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index fd73e4ce431..25ff19a6e4
Re: [PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268]
Harald,
Thank you for reviewing my code. I've been doing research and debugging to
investigate the error thrown by Intel and NAG for the deferred parameter in
the dummy variable declaration. I found where the problem was and added the
fix as part of my patch. I've attached the patch as a file, which also
includes your feedback and suggested fixes. I've updated the test case
pdt_37.f03 to check for the POINTER or ALLOCATABLE error as you suggested.
All regression tests pass, including the new ones, after including the fix
for the POINTER or ALLOCATABLE error for CLASS declarations of PDTs when
deferred length parameters are used. This was tested on WSL 2, with Ubuntu
20.04 distro.
Is this okay to push to the trunk?
Thanks,
Alexander Westbrooks
On Sun, Feb 11, 2024 at 2:11 PM Harald Anlauf wrote:
> Hi Alex,
>
> I've been unable to apply your patch to my local trunk, likely due to
> whitespace issues my newsreader handles differently from your site.
> I see it inline instead of attached.
>
> A few general remarks:
>
> Please follow the general recommendation regarding style if possible,
> see https://www.gnu.org/prep/standards/standards.html#Formatting
> regarding formatting/whitespace use (5.1) and comments (5.2)
>
> Also, when an error message text spans multiple lines, please place the
> whitespace at the end of a line, not at the beginning of the new one:
>
> > + if ( resolve_bindings_derived->attr.pdt_template &&
> > + !gfc_pdt_is_instance_of(resolve_bindings_derived,
> > + CLASS_DATA(me_arg)->ts.u.derived))
> > +{
> > + gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of"
> > +" the parametric derived-type %qs", me_arg->name, proc->name,
>
>gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of "
> "the parametric derived-type %qs", me_arg->name,
> proc->name,
>
> > +me_arg->name, &where, resolve_bindings_derived->name);
> > + goto error;
> > +}
>
> The following change is almost unreadable: the lnegthy comment is split
> over three parts and almost hides the code. Couldn't this be combined
> into one comment before the function?
>
> > diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
> > index fddf68f8398..11f4bac0415 100644
> > --- a/gcc/fortran/symbol.cc
> > +++ b/gcc/fortran/symbol.cc
> > @@ -5172,6 +5172,35 @@ gfc_type_is_extension_of (gfc_symbol *t1,
> gfc_symbol
> > *t2)
> > return gfc_compare_derived_types (t1, t2);
> > }
> >
> > +/* Check if a parameterized derived type t2 is an instance of a PDT
> > template t1 */
> > +
> > +bool
> > +gfc_pdt_is_instance_of(gfc_symbol *t1, gfc_symbol *t2)
> > +{
> > + if ( !t1->attr.pdt_template || !t2->attr.pdt_type )
> > +return false;
> > +
> > + /*
> > +in decl.cc, gfc_get_pdt_instance, a pdt instance is given a 3
> > character prefix "Pdt", followed
> > +by an underscore list of the kind parameters, up to a maximum of 8.
> > +
> > +So to check if a PDT Type corresponds to the template, extract the
> > core derive_type name,
> > +and then see if it is type compatible by name...
> > +
> > +For example:
> > +
> > +Pdtf_2_2 -> extract out the 'f' -> see if the derived type 'f' is
> > compatible with symbol t1
> > + */
> > +
> > + // Starting at index 3 of the string in order to skip past the 'Pdt'
> > prefix
> > + // Also, here the length of the template name is used in order to
> avoid
> > the
> > + // kind parameter suffixes that are placed at the end of PDT instance
> > names.
> > + if ( !(strncmp(&(t2->name[3]), t1->name, strlen(t1->name)) == 0) )
> > +return false;
> > +
> > + return true;
> > +}
> > +
> >
> > /* Check if two typespecs are type compatible (F03:5.1.1.2):
> > If ts1 is nonpolymorphic, ts2 must be the same type.
>
> The following testcase tests for errors. I tried Intel and NAG on it
> after commenting the 'contains' section of the type desclaration.
> Both complained about subroutine deferred_len_param, e.g.
>
> Intel:
> A colon may only be used as a type parameter value in the declaration of
> an object that has the POINTER or ALLOCATABLE attribute. [THIS]
> class(param_deriv_type(:)), intent(inout) :: this
>
> NAG:
> Entity THIS of type PARAM_DERIV_TYPE(A=:) has a deferred length type
> parameter but
Re: [PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268]
Harald,
Jerry helped me figure out my editor settings so that I could fix
whitespace and formatting issues in my code. With my editor configured
correctly, I saw that my code was not conforming to coding standards
as I previously thought it was. I have fixed those things and updated
my patch. Thank you for your patience.
Let me know if this is okay to push to the trunk.
Thanks,
Alexander Westbrooks
On Sun, Feb 25, 2024 at 2:40 PM Alexander Westbrooks
wrote:
>
> Harald,
>
> Thank you for reviewing my code. I've been doing research and debugging to
> investigate the error thrown by Intel and NAG for the deferred parameter in
> the dummy variable declaration. I found where the problem was and added the
> fix as part of my patch. I've attached the patch as a file, which also
> includes your feedback and suggested fixes. I've updated the test case
> pdt_37.f03 to check for the POINTER or ALLOCATABLE error as you suggested.
>
> All regression tests pass, including the new ones, after including the fix
> for the POINTER or ALLOCATABLE error for CLASS declarations of PDTs when
> deferred length parameters are used. This was tested on WSL 2, with Ubuntu
> 20.04 distro.
>
> Is this okay to push to the trunk?
>
> Thanks,
>
> Alexander Westbrooks
>
>
> On Sun, Feb 11, 2024 at 2:11 PM Harald Anlauf wrote:
>>
>> Hi Alex,
>>
>> I've been unable to apply your patch to my local trunk, likely due to
>> whitespace issues my newsreader handles differently from your site.
>> I see it inline instead of attached.
>>
>> A few general remarks:
>>
>> Please follow the general recommendation regarding style if possible,
>> see https://www.gnu.org/prep/standards/standards.html#Formatting
>> regarding formatting/whitespace use (5.1) and comments (5.2)
>>
>> Also, when an error message text spans multiple lines, please place the
>> whitespace at the end of a line, not at the beginning of the new one:
>>
>> > + if ( resolve_bindings_derived->attr.pdt_template &&
>> > + !gfc_pdt_is_instance_of(resolve_bindings_derived,
>> > + CLASS_DATA(me_arg)->ts.u.derived))
>> > +{
>> > + gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of"
>> > +" the parametric derived-type %qs", me_arg->name, proc->name,
>>
>>gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of "
>> "the parametric derived-type %qs", me_arg->name,
>> proc->name,
>>
>> > +me_arg->name, &where, resolve_bindings_derived->name);
>> > + goto error;
>> > +}
>>
>> The following change is almost unreadable: the lnegthy comment is split
>> over three parts and almost hides the code. Couldn't this be combined
>> into one comment before the function?
>>
>> > diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
>> > index fddf68f8398..11f4bac0415 100644
>> > --- a/gcc/fortran/symbol.cc
>> > +++ b/gcc/fortran/symbol.cc
>> > @@ -5172,6 +5172,35 @@ gfc_type_is_extension_of (gfc_symbol *t1, gfc_symbol
>> > *t2)
>> > return gfc_compare_derived_types (t1, t2);
>> > }
>> >
>> > +/* Check if a parameterized derived type t2 is an instance of a PDT
>> > template t1 */
>> > +
>> > +bool
>> > +gfc_pdt_is_instance_of(gfc_symbol *t1, gfc_symbol *t2)
>> > +{
>> > + if ( !t1->attr.pdt_template || !t2->attr.pdt_type )
>> > +return false;
>> > +
>> > + /*
>> > +in decl.cc, gfc_get_pdt_instance, a pdt instance is given a 3
>> > character prefix "Pdt", followed
>> > +by an underscore list of the kind parameters, up to a maximum of 8.
>> > +
>> > +So to check if a PDT Type corresponds to the template, extract the
>> > core derive_type name,
>> > +and then see if it is type compatible by name...
>> > +
>> > +For example:
>> > +
>> > +Pdtf_2_2 -> extract out the 'f' -> see if the derived type 'f' is
>> > compatible with symbol t1
>> > + */
>> > +
>> > + // Starting at index 3 of the string in order to skip past the 'Pdt'
>> > prefix
>> > + // Also, here the length of the template name is used in order to avoid
>> > the
>> > + // kind parameter suffixes that are placed at the end of PDT instance
>> > names.
>> > + if ( !(strncmp(&(
Re: [PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268]
Hello,
I've updated the patch with those changes, ran through the gcc-verify
step and fixed up the commit, and then pushed it to the trunk.
Thank you for your feedback, and I look forward to working on GFortran.
Thanks,
Alexander Westbrooks
On Wed, Feb 28, 2024 at 1:55 PM Harald Anlauf wrote:
>
> Hi Alex,
>
> this is now mostly correct, with the following exceptions:
>
> First, you should notice that the formatting of the commit message,
> when checked using "git gcc-verify", needs minor corrections. You
> will be guided how to fix this yourself.
>
> Second, testcase pdt_37.f03 has an undeclared dummy argument, which
> can be detected by adding "implicit none" (I usually use that
> whenever implicit typing is not wanted explicitly). I would get:
>
> pdt_37.f03:33:47:
>
> 33 | subroutine assumed_len_param_ptr(this, that)
>| 1
> Error: Symbol 'that' at (1) has no IMPLICIT type; did you mean 'this'?
>
> I assume you want to uncomment the declaration of dummy 'that'.
>
> Third, I still see a - minor - indentation/tabbing/space issue here:
>
> diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
> index 44f89f6afb4..852e0820e6a 100644
> --- a/gcc/fortran/resolve.cc
> +++ b/gcc/fortran/resolve.cc
> [...]
> + if ( resolve_bindings_derived->attr.pdt_template
> + && gfc_pdt_is_instance_of (resolve_bindings_derived,
> + CLASS_DATA (me_arg)->ts.u.derived)
> + && (me_arg->param_list != NULL)
> + && (gfc_spec_list_type (me_arg->param_list,
> + CLASS_DATA(me_arg)->ts.u.derived)
> +!= SPEC_ASSUMED))
>
> OK with the above fixed.
>
> Thanks for the patch!
>
> Harald
>
> On 2/28/24 07:24, Alexander Westbrooks wrote:
> > Harald,
> >
> > Jerry helped me figure out my editor settings so that I could fix
> > whitespace and formatting issues in my code. With my editor configured
> > correctly, I saw that my code was not conforming to coding standards
> > as I previously thought it was. I have fixed those things and updated
> > my patch. Thank you for your patience.
> >
> > Let me know if this is okay to push to the trunk.
> >
> > Thanks,
> >
> > Alexander Westbrooks
> >
> > On Sun, Feb 25, 2024 at 2:40 PM Alexander Westbrooks
> > wrote:
> >>
> >> Harald,
> >>
> >> Thank you for reviewing my code. I've been doing research and debugging to
> >> investigate the error thrown by Intel and NAG for the deferred parameter
> >> in the dummy variable declaration. I found where the problem was and added
> >> the fix as part of my patch. I've attached the patch as a file, which also
> >> includes your feedback and suggested fixes. I've updated the test case
> >> pdt_37.f03 to check for the POINTER or ALLOCATABLE error as you suggested.
> >>
> >> All regression tests pass, including the new ones, after including the fix
> >> for the POINTER or ALLOCATABLE error for CLASS declarations of PDTs when
> >> deferred length parameters are used. This was tested on WSL 2, with Ubuntu
> >> 20.04 distro.
> >>
> >> Is this okay to push to the trunk?
> >>
> >> Thanks,
> >>
> >> Alexander Westbrooks
> >>
> >>
> >> On Sun, Feb 11, 2024 at 2:11 PM Harald Anlauf wrote:
> >>>
> >>> Hi Alex,
> >>>
> >>> I've been unable to apply your patch to my local trunk, likely due to
> >>> whitespace issues my newsreader handles differently from your site.
> >>> I see it inline instead of attached.
> >>>
> >>> A few general remarks:
> >>>
> >>> Please follow the general recommendation regarding style if possible,
> >>> see https://www.gnu.org/prep/standards/standards.html#Formatting
> >>> regarding formatting/whitespace use (5.1) and comments (5.2)
> >>>
> >>> Also, when an error message text spans multiple lines, please place the
> >>> whitespace at the end of a line, not at the beginning of the new one:
> >>>
> >>>> + if ( resolve_bindings_derived->attr.pdt_template &&
> >>>> + !gfc_pdt_is_instance_of(resolve_bindings_derived,
> >>>> + CLASS_DATA(me_arg)->ts.u.derived))
> >>>> +{
> >>>> +
Re: [PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268]
Hello, I meant to add a link to the commit to the previous email: https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=edfe198084338691d0facc86bf8dfa6ede3ca676 Thanks, Alexander Westbrooks On Wed, Feb 28, 2024 at 8:24 PM Alexander Westbrooks wrote: > > Hello, > > I've updated the patch with those changes, ran through the gcc-verify > step and fixed up the commit, and then pushed it to the trunk. > > Thank you for your feedback, and I look forward to working on GFortran. > > Thanks, > > Alexander Westbrooks > > On Wed, Feb 28, 2024 at 1:55 PM Harald Anlauf wrote: > > > > Hi Alex, > > > > this is now mostly correct, with the following exceptions: > > > > First, you should notice that the formatting of the commit message, > > when checked using "git gcc-verify", needs minor corrections. You > > will be guided how to fix this yourself. > > > > Second, testcase pdt_37.f03 has an undeclared dummy argument, which > > can be detected by adding "implicit none" (I usually use that > > whenever implicit typing is not wanted explicitly). I would get: > > > > pdt_37.f03:33:47: > > > > 33 | subroutine assumed_len_param_ptr(this, that) > >| 1 > > Error: Symbol 'that' at (1) has no IMPLICIT type; did you mean 'this'? > > > > I assume you want to uncomment the declaration of dummy 'that'. > > > > Third, I still see a - minor - indentation/tabbing/space issue here: > > > > diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc > > index 44f89f6afb4..852e0820e6a 100644 > > --- a/gcc/fortran/resolve.cc > > +++ b/gcc/fortran/resolve.cc > > [...] > > + if ( resolve_bindings_derived->attr.pdt_template > > + && gfc_pdt_is_instance_of (resolve_bindings_derived, > > + CLASS_DATA (me_arg)->ts.u.derived) > > + && (me_arg->param_list != NULL) > > + && (gfc_spec_list_type (me_arg->param_list, > > +CLASS_DATA(me_arg)->ts.u.derived) > > +!= SPEC_ASSUMED)) > > > > OK with the above fixed. > > > > Thanks for the patch! > > > > Harald > > > > On 2/28/24 07:24, Alexander Westbrooks wrote: > > > Harald, > > > > > > Jerry helped me figure out my editor settings so that I could fix > > > whitespace and formatting issues in my code. With my editor configured > > > correctly, I saw that my code was not conforming to coding standards > > > as I previously thought it was. I have fixed those things and updated > > > my patch. Thank you for your patience. > > > > > > Let me know if this is okay to push to the trunk. > > > > > > Thanks, > > > > > > Alexander Westbrooks > > > > > > On Sun, Feb 25, 2024 at 2:40 PM Alexander Westbrooks > > > wrote: > > >> > > >> Harald, > > >> > > >> Thank you for reviewing my code. I've been doing research and debugging > > >> to investigate the error thrown by Intel and NAG for the deferred > > >> parameter in the dummy variable declaration. I found where the problem > > >> was and added the fix as part of my patch. I've attached the patch as a > > >> file, which also includes your feedback and suggested fixes. I've > > >> updated the test case pdt_37.f03 to check for the POINTER or ALLOCATABLE > > >> error as you suggested. > > >> > > >> All regression tests pass, including the new ones, after including the > > >> fix for the POINTER or ALLOCATABLE error for CLASS declarations of PDTs > > >> when deferred length parameters are used. This was tested on WSL 2, with > > >> Ubuntu 20.04 distro. > > >> > > >> Is this okay to push to the trunk? > > >> > > >> Thanks, > > >> > > >> Alexander Westbrooks > > >> > > >> > > >> On Sun, Feb 11, 2024 at 2:11 PM Harald Anlauf wrote: > > >>> > > >>> Hi Alex, > > >>> > > >>> I've been unable to apply your patch to my local trunk, likely due to > > >>> whitespace issues my newsreader handles differently from your site. > > >>> I see it inline instead of attached. > > >>> > > >>> A few general remarks: >
Re: PR82943 - Suggested patch to fix
Hello and Happy New Year! I wanted to follow up on this patch I made to address PR82943 for GFortran. Has anyone had a chance to review it? Thanks, Alexander Westbrooks On Thu, Jun 29, 2023 at 10:38 PM Alexander Westbrooks wrote: > Hello, > > I have finished my testing, and updated my patch and relevant Changelogs. > I added 4 new tests and all the existing tests in the current testsuite > for gfortran passed or failed as expected. Do I need to attach the test > results here? > > The platform I tested on was a Docker container running in Docker Desktop, > running the "mcr.microsoft.com/devcontainers/universal:2-linux" image. > > I also made sure that my code changes followed the coding standards. > Please let me know if there is anything else that I need to do. I don't > have write-access to the repository. > > Thanks, > > Alexander > > On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf wrote: > >> Hi Alex, >> >> welcome to the gfortran community. It is great that you are trying >> to get actively involved. >> >> You already did quite a few things right: patches shall be sent to >> the gcc-patches ML, but Fortran reviewers usually notice them only >> where they are copied to the fortran ML. >> >> There are some general recommendations on the formatting of C code, >> like indentation, of the patches, and of the commit log entries. >> >> Regarding coding standards, see https://www.gnu.org/prep/standards/ . >> >> Regarding testcases, a recommendation is to have a look at >> existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then >> decide if the testcase shall test the compile-time or run-time >> behaviour, and add the necessary dejagnu directives. >> >> You should also verify if your patch passes regression testing. >> For changes to gfortran, it is usually sufficient to run >> >> make check-fortran -j >> >> where is the number of parallel tests. >> You would need to report also the platform where you tested on. >> >> There is also a legal issue to consider before non-trivial patches can >> be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal >> >> If your patch is accepted and if you do not have write-access to the >> repository, one of the maintainers will likely take care of it. >> If you become a regular contributor, you will probably want to consider >> getting write access. >> >> Cheers, >> Harald >> >> >> >> On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote: >> > Hello, >> > >> > I am new to the GFortran community. Over the past two weeks I created a >> > patch that should fix PR82943 for GFortran. I have attached it to this >> > email. The patch allows the code below to compile successfully. I am >> > working on creating test cases next, but I am new to the process so it >> may >> > take me some time. After I make test cases, do I email them to you as >> well? >> > Do I need to make a pull-request on github in order to get the patch >> > reviewed? >> > >> > Thank you, >> > >> > Alexander Westbrooks >> > >> > module testmod >> > >> > public :: foo >> > >> > type, public :: tough_lvl_0(a, b) >> > integer, kind :: a = 1 >> > integer, len :: b >> > contains >> > procedure :: foo >> > end type >> > >> > type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c) >> > integer, len :: c >> > contains >> > procedure :: bar >> > end type >> > >> > type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d) >> > integer, len :: d >> > contains >> > procedure :: foobar >> > end type >> > >> > contains >> > subroutine foo(this) >> > class(tough_lvl_0(1,*)), intent(inout) :: this >> > end subroutine >> > >> > subroutine bar(this) >> > class(tough_lvl_1(1,*,*)), intent(inout) :: this >> > end subroutine >> > >> > subroutine foobar(this) >> > class(tough_lvl_2(1,*,*,*)), intent(inout) :: this >> > end subroutine >> > >> > end module >> > >> > PROGRAM testprogram >> > USE testmod >> > >> > TYPE(tough_lvl_0(1,5)) :: test_pdt_0 >> > TYPE(tough_lvl_1(1,5,6)) :: test_pdt_1 >> > TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2 >> > >> > CALL test_pdt_0%foo() >> > >> > CALL test_pdt_1%foo() >> > CALL test_pdt_1%bar() >> > >> > CALL test_pdt_2%foo() >> > CALL test_pdt_2%bar() >> > CALL test_pdt_2%foobar() >> > >> > >> > END PROGRAM testprogram >> >>
Re: PR82943 - Suggested patch to fix
Based on what I am reading here, I can either do the DCO path or the copy right form path? Or is it both, where I send in the copy right forms and then on every commit I put the DCO? On Sat, Jan 20, 2024 at 3:40 PM Harald Anlauf wrote: > Am 20.01.24 um 21:37 schrieb Jerry D: > > On 1/20/24 12:08 PM, Harald Anlauf wrote: > >> Am 20.01.24 um 20:08 schrieb Jerry D: > >>> On 1/20/24 10:46 AM, Alexander Westbrooks wrote: > >>>> Hello and Happy New Year! > >>>> > >>>> I wanted to follow up on this patch I made to address PR82943 for > >>>> GFortran. Has anyone had a chance to review it? > >>>> > >>>> Thanks, > >>>> > >>>> Alexander Westbrooks > >>>> > >>> > >>> Inserting myself in here just a little bit. I will apply and test > today > >>> if I can. Paul is unavailable for a few weeks. Harald can chime in. > >>> > >>> Do you have commit rights for gcc? > >> > >> I am not aware of Alex having a copyright assignment on file, > >> or a DCO certificate, and the patch is not signed off. > >> But I might be wrong. > >> > > --- snip --- > > > > I do not mind committing this but need clarifications regarding the > > copyright (copyleft?) rules in this case. In the past we have allowed > > small contributions like this. This may be a little more than minor. > > It is. This is why I pointed to: > > https://gcc.gnu.org/dco.html > > > Regardless it appears to do the job! > > > > Jerry > > > > > >
Re: PR82943 - Suggested patch to fix
Hello, I wanted to follow up on this, and ask what the next steps would be to incorporate this patch? Thanks, Alexander Westbrooks On Thu, Jun 29, 2023 at 10:38 PM Alexander Westbrooks wrote: > Hello, > > I have finished my testing, and updated my patch and relevant Changelogs. > I added 4 new tests and all the existing tests in the current testsuite > for gfortran passed or failed as expected. Do I need to attach the test > results here? > > The platform I tested on was a Docker container running in Docker Desktop, > running the "mcr.microsoft.com/devcontainers/universal:2-linux" image. > > I also made sure that my code changes followed the coding standards. > Please let me know if there is anything else that I need to do. I don't > have write-access to the repository. > > Thanks, > > Alexander > > On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf wrote: > >> Hi Alex, >> >> welcome to the gfortran community. It is great that you are trying >> to get actively involved. >> >> You already did quite a few things right: patches shall be sent to >> the gcc-patches ML, but Fortran reviewers usually notice them only >> where they are copied to the fortran ML. >> >> There are some general recommendations on the formatting of C code, >> like indentation, of the patches, and of the commit log entries. >> >> Regarding coding standards, see https://www.gnu.org/prep/standards/ . >> >> Regarding testcases, a recommendation is to have a look at >> existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then >> decide if the testcase shall test the compile-time or run-time >> behaviour, and add the necessary dejagnu directives. >> >> You should also verify if your patch passes regression testing. >> For changes to gfortran, it is usually sufficient to run >> >> make check-fortran -j >> >> where is the number of parallel tests. >> You would need to report also the platform where you tested on. >> >> There is also a legal issue to consider before non-trivial patches can >> be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal >> >> If your patch is accepted and if you do not have write-access to the >> repository, one of the maintainers will likely take care of it. >> If you become a regular contributor, you will probably want to consider >> getting write access. >> >> Cheers, >> Harald >> >> >> >> On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote: >> > Hello, >> > >> > I am new to the GFortran community. Over the past two weeks I created a >> > patch that should fix PR82943 for GFortran. I have attached it to this >> > email. The patch allows the code below to compile successfully. I am >> > working on creating test cases next, but I am new to the process so it >> may >> > take me some time. After I make test cases, do I email them to you as >> well? >> > Do I need to make a pull-request on github in order to get the patch >> > reviewed? >> > >> > Thank you, >> > >> > Alexander Westbrooks >> > >> > module testmod >> > >> > public :: foo >> > >> > type, public :: tough_lvl_0(a, b) >> > integer, kind :: a = 1 >> > integer, len :: b >> > contains >> > procedure :: foo >> > end type >> > >> > type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c) >> > integer, len :: c >> > contains >> > procedure :: bar >> > end type >> > >> > type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d) >> > integer, len :: d >> > contains >> > procedure :: foobar >> > end type >> > >> > contains >> > subroutine foo(this) >> > class(tough_lvl_0(1,*)), intent(inout) :: this >> > end subroutine >> > >> > subroutine bar(this) >> > class(tough_lvl_1(1,*,*)), intent(inout) :: this >> > end subroutine >> > >> > subroutine foobar(this) >> > class(tough_lvl_2(1,*,*,*)), intent(inout) :: this >> > end subroutine >> > >> > end module >> > >> > PROGRAM testprogram >> > USE testmod >> > >> > TYPE(tough_lvl_0(1,5)) :: test_pdt_0 >> > TYPE(tough_lvl_1(1,5,6)) :: test_pdt_1 >> > TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2 >> > >> > CALL test_pdt_0%foo() >> > >> > CALL test_pdt_1%foo() >> > CALL test_pdt_1%bar() >> > >> > CALL test_pdt_2%foo() >> > CALL test_pdt_2%bar() >> > CALL test_pdt_2%foobar() >> > >> > >> > END PROGRAM testprogram >> >>
PR82943 - Suggested patch to fix
Hello, I am new to the GFortran community. Over the past two weeks I created a patch that should fix PR82943 for GFortran. I have attached it to this email. The patch allows the code below to compile successfully. I am working on creating test cases next, but I am new to the process so it may take me some time. After I make test cases, do I email them to you as well? Do I need to make a pull-request on github in order to get the patch reviewed? Thank you, Alexander Westbrooks module testmod public :: foo type, public :: tough_lvl_0(a, b) integer, kind :: a = 1 integer, len :: b contains procedure :: foo end type type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c) integer, len :: c contains procedure :: bar end type type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d) integer, len :: d contains procedure :: foobar end type contains subroutine foo(this) class(tough_lvl_0(1,*)), intent(inout) :: this end subroutine subroutine bar(this) class(tough_lvl_1(1,*,*)), intent(inout) :: this end subroutine subroutine foobar(this) class(tough_lvl_2(1,*,*,*)), intent(inout) :: this end subroutine end module PROGRAM testprogram USE testmod TYPE(tough_lvl_0(1,5)) :: test_pdt_0 TYPE(tough_lvl_1(1,5,6)) :: test_pdt_1 TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2 CALL test_pdt_0%foo() CALL test_pdt_1%foo() CALL test_pdt_1%bar() CALL test_pdt_2%foo() CALL test_pdt_2%bar() CALL test_pdt_2%foobar() END PROGRAM testprogram 0001-bug-patch-PR82943.patch Description: Binary data
Re: PR82943 - Suggested patch to fix
Hello, I have finished my testing, and updated my patch and relevant Changelogs. I added 4 new tests and all the existing tests in the current testsuite for gfortran passed or failed as expected. Do I need to attach the test results here? The platform I tested on was a Docker container running in Docker Desktop, running the "mcr.microsoft.com/devcontainers/universal:2-linux" image. I also made sure that my code changes followed the coding standards. Please let me know if there is anything else that I need to do. I don't have write-access to the repository. Thanks, Alexander On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf wrote: > Hi Alex, > > welcome to the gfortran community. It is great that you are trying > to get actively involved. > > You already did quite a few things right: patches shall be sent to > the gcc-patches ML, but Fortran reviewers usually notice them only > where they are copied to the fortran ML. > > There are some general recommendations on the formatting of C code, > like indentation, of the patches, and of the commit log entries. > > Regarding coding standards, see https://www.gnu.org/prep/standards/ . > > Regarding testcases, a recommendation is to have a look at > existing testcases, e.g. in gcc/testsuite/gfortran.dg/, and then > decide if the testcase shall test the compile-time or run-time > behaviour, and add the necessary dejagnu directives. > > You should also verify if your patch passes regression testing. > For changes to gfortran, it is usually sufficient to run > > make check-fortran -j > > where is the number of parallel tests. > You would need to report also the platform where you tested on. > > There is also a legal issue to consider before non-trivial patches can > be accepted for incorporation: https://gcc.gnu.org/contribute.html#legal > > If your patch is accepted and if you do not have write-access to the > repository, one of the maintainers will likely take care of it. > If you become a regular contributor, you will probably want to consider > getting write access. > > Cheers, > Harald > > > > On 6/24/23 19:17, Alexander Westbrooks via Gcc-patches wrote: > > Hello, > > > > I am new to the GFortran community. Over the past two weeks I created a > > patch that should fix PR82943 for GFortran. I have attached it to this > > email. The patch allows the code below to compile successfully. I am > > working on creating test cases next, but I am new to the process so it > may > > take me some time. After I make test cases, do I email them to you as > well? > > Do I need to make a pull-request on github in order to get the patch > > reviewed? > > > > Thank you, > > > > Alexander Westbrooks > > > > module testmod > > > > public :: foo > > > > type, public :: tough_lvl_0(a, b) > > integer, kind :: a = 1 > > integer, len :: b > > contains > > procedure :: foo > > end type > > > > type, public, EXTENDS(tough_lvl_0) :: tough_lvl_1 (c) > > integer, len :: c > > contains > > procedure :: bar > > end type > > > > type, public, EXTENDS(tough_lvl_1) :: tough_lvl_2 (d) > > integer, len :: d > > contains > > procedure :: foobar > > end type > > > > contains > > subroutine foo(this) > > class(tough_lvl_0(1,*)), intent(inout) :: this > > end subroutine > > > > subroutine bar(this) > > class(tough_lvl_1(1,*,*)), intent(inout) :: this > > end subroutine > > > > subroutine foobar(this) > > class(tough_lvl_2(1,*,*,*)), intent(inout) :: this > > end subroutine > > > > end module > > > > PROGRAM testprogram > > USE testmod > > > > TYPE(tough_lvl_0(1,5)) :: test_pdt_0 > > TYPE(tough_lvl_1(1,5,6)) :: test_pdt_1 > > TYPE(tough_lvl_2(1,5,6,7)) :: test_pdt_2 > > > > CALL test_pdt_0%foo() > > > > CALL test_pdt_1%foo() > > CALL test_pdt_1%bar() > > > > CALL test_pdt_2%foo() > > CALL test_pdt_2%bar() > > CALL test_pdt_2%foobar() > > > > > > END PROGRAM testprogram > > 0001-Fix-fortran-PR82943-PR86148-and-PR86268.patch Description: Binary data
