Re: PR82943 - Suggested patch to fix

[Paul Richard Thomas <paul.richard.thomas@gmail.com>]

Hi Alexander,

I suggest that you take a look at PR82649 before going too far down
the road of fixing PDT bugs. This PR underlines just how wrong the PDT
representation is - mea culpa!

The mechanics for constructing PDTs are in
decl.cc(gfc_get_pdt_instance). They need to be turned inside out to
create a container, not unlike the class containers, with
{data-field(assumed rank array?), kind and len parameters}. This will
then trigger all manner of failures in trans-***.cc in particular.

It had been my intention to turn to PDTs after I complete my scourge
of associate construct bugs. If you want to take this on, please do so
and I will give you all the help that I can. You will see from PR82649
that I have been promising to get on to this for a long time but have
not had the time thus far :-( If you want to get on with parsing bugs
to start with, please be my guest!

I notice that searching for PDT in PR title lines generates 48 hits,
while the PDT meta-bug PR82173 only has 28 blockers. I will get on
with the housekeeping this weekend by updating PR82173 and eliminating
duplicates.

Welcome, Alexander!

Paul

On Fri, 30 Jun 2023 at 05:42, Steve Kargl via Fortran
<fortran@gcc.gnu.org> wrote:
>
> On Thu, Jun 29, 2023 at 10:38:42PM -0500, Alexander Westbrooks via Fortran wrote:
> > 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?
>
> Yes.  It helps others also do testing to have one self-contained
> patch (which I don't know to generate with git and new files :-( ).
> It may also be a good idea to attach the patch and test cases to
> the PR in bugzilla so that they don't accidentally get lost.
>
> > 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.
>
> See the legal link that Harald provided.  At one time, one needed to
> assign copyright to the FSF with a wet-ink signature on some form.
> Now, I think you just need to attest that you have the right to
> provide the code to the gcc project.
>
> PS: Welcome to the gfortran development world.  Don't be put off
> if there is a delay in getting feedback/review.  There are too
> few contributors and too little time.   If a week passes simply
> ping the mailing list.  I'll try to carve out some time to look
> over your patch this weekend.
>
> --
> steve
>
>
> >
> > Thanks,
> >
> > Alexander
> >
> > On Wed, Jun 28, 2023 at 4:14 PM Harald Anlauf <anlauf@gmx.de> 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 <n>
> > >
> > > where <n> 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
> > >
> > >
>
>
>
> --
> Steve



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein