public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Harald Anlauf <anlauf@gmx.de>
To: Paul Richard Thomas <paul.richard.thomas@gmail.com>
Cc: "fortran@gcc.gnu.org" <fortran@gcc.gnu.org>,
	gcc-patches <gcc-patches@gcc.gnu.org>,
	Steve Kargl <sgk@troutmask.apl.washington.edu>
Subject: Re: [Patch, fortran] PR107900 Select type with intrinsic type inside associate causes ICE / Segmenation fault
Date: Wed, 21 Jun 2023 20:40:03 +0200	[thread overview]
Message-ID: <af7bfd03-3e6c-e869-b1c6-7d89bbdc9732@gmx.de> (raw)
In-Reply-To: <CAGkQGiKd8=XFTCA0im5YP6HvkMaQcy3Gtojt=0G72bwnh3hu0A@mail.gmail.com>

Hi Paul,

while I only had a minor question regarding gfc_is_ptr_fcn(),
can you still try to enlighten me why that second part
was necessary?  (I believed it to be redundant and may have
overlooked the obvious.)

Cheers,
Harald

On 6/21/23 18:12, Paul Richard Thomas via Gcc-patches wrote:
> Committed as r14-2022-g577223aebc7acdd31e62b33c1682fe54a622ae27
>
> Thanks for the help and the review Harald. Thanks to Steve too for
> picking up Neil Carlson's bugs.
>
> Cheers
>
> Paul
>
> On Tue, 20 Jun 2023 at 22:57, Harald Anlauf <anlauf@gmx.de> wrote:
>>
>> Hi Paul,
>>
>> On 6/20/23 12:54, Paul Richard Thomas via Gcc-patches wrote:
>>> Hi Harald,
>>>
>>> Fixing the original testcase in this PR turned out to be slightly more
>>> involved than I expected. However, it resulted in an open door to fix
>>> some other PRs and the attached much larger patch.
>>>
>>> This time, I did remember to include the testcases in the .diff :-)
>>
>> indeed! :-)
>>
>> I've only had a superficial look so far although it looks very good.
>> (I have to trust your experience with unlimited polymorphism.)
>>
>> However, I was wondering about the following helper function:
>>
>> +bool
>> +gfc_is_ptr_fcn (gfc_expr *e)
>> +{
>> +  return e != NULL && e->expr_type == EXPR_FUNCTION
>> +             && (gfc_expr_attr (e).pointer
>> +                 || (e->ts.type == BT_CLASS
>> +                     && CLASS_DATA (e)->attr.class_pointer));
>> +}
>> +
>> +
>>    /* Copy a shape array.  */
>>
>> Is there a case where gfc_expr_attr (e).pointer returns false
>> and you really need the || part?  Looking at gfc_expr_attr
>> and the present context, it might just not be necessary.
>>
>>> I believe that, between the Change.Logs and the comments, it is
>>> reasonably self-explanatory.
>>>
>>> OK for trunk?
>>
>> OK from my side.
>>
>> Thanks for the patch!
>>
>> Harald
>>
>>> Regards
>>>
>>> Paul
>>>
>>> Fortran: Fix some bugs in associate [PR87477]
>>>
>>> 2023-06-20  Paul Thomas  <pault@gcc.gnu.org>
>>>
>>> gcc/fortran
>>> PR fortran/87477
>>> PR fortran/88688
>>> PR fortran/94380
>>> PR fortran/107900
>>> PR fortran/110224
>>> * decl.cc (char_len_param_value): Fix memory leak.
>>> (resolve_block_construct): Remove unnecessary static decls.
>>> * expr.cc (gfc_is_ptr_fcn): New function.
>>> (gfc_check_vardef_context): Use it to permit pointer function
>>> result selectors to be used for associate names in variable
>>> definition context.
>>> * gfortran.h: Prototype for gfc_is_ptr_fcn.
>>> * match.cc (build_associate_name): New function.
>>> (gfc_match_select_type): Use the new function to replace inline
>>> version and to build a new associate name for the case where
>>> the supplied associate name is already used for that purpose.
>>> * resolve.cc (resolve_assoc_var): Call gfc_is_ptr_fcn to allow
>>> associate names with pointer function targets to be used in
>>> variable definition context.
>>> * trans-decl.cc (gfc_get_symbol_decl): Unlimited polymorphic
>>> variables need deferred initialisation of the vptr.
>>> (gfc_trans_deferred_vars): Do the vptr initialisation.
>>> * trans-stmt.cc (trans_associate_var): Ensure that a pointer
>>> associate name points to the target of the selector and not
>>> the selector itself.
>>>
>>> gcc/testsuite/
>>> PR fortran/87477
>>> PR fortran/107900
>>> * gfortran.dg/pr107900.f90 : New test
>>>
>>> PR fortran/110224
>>> * gfortran.dg/pr110224.f90 : New test
>>>
>>> PR fortran/88688
>>> * gfortran.dg/pr88688.f90 : New test
>>>
>>> PR fortran/94380
>>> * gfortran.dg/pr94380.f90 : New test
>>>
>>> PR fortran/95398
>>> * gfortran.dg/pr95398.f90 : Set -std=f2008, bump the line
>>> numbers in the error tests by two and change the text in two.
>>
>
>


WARNING: multiple messages have this Message-ID
From: Harald Anlauf <anlauf@gmx.de>
To: gcc-patches@gcc.gnu.org
Cc: fortran@gcc.gnu.org
Subject: Re: [Patch, fortran] PR107900 Select type with intrinsic type inside associate causes ICE / Segmenation fault
Date: Wed, 21 Jun 2023 20:40:03 +0200	[thread overview]
Message-ID: <af7bfd03-3e6c-e869-b1c6-7d89bbdc9732@gmx.de> (raw)
Message-ID: <20230621184003.aIRoCpC4z5kre_0ptf2tmotVh1c9ox4wyyNP15sDra4@z> (raw)
In-Reply-To: <CAGkQGiKd8=XFTCA0im5YP6HvkMaQcy3Gtojt=0G72bwnh3hu0A@mail.gmail.com>

Hi Paul,

while I only had a minor question regarding gfc_is_ptr_fcn(),
can you still try to enlighten me why that second part
was necessary?  (I believed it to be redundant and may have
overlooked the obvious.)

Cheers,
Harald

On 6/21/23 18:12, Paul Richard Thomas via Gcc-patches wrote:
> Committed as r14-2022-g577223aebc7acdd31e62b33c1682fe54a622ae27
> 
> Thanks for the help and the review Harald. Thanks to Steve too for
> picking up Neil Carlson's bugs.
> 
> Cheers
> 
> Paul
> 
> On Tue, 20 Jun 2023 at 22:57, Harald Anlauf <anlauf@gmx.de> wrote:
>>
>> Hi Paul,
>>
>> On 6/20/23 12:54, Paul Richard Thomas via Gcc-patches wrote:
>>> Hi Harald,
>>>
>>> Fixing the original testcase in this PR turned out to be slightly more
>>> involved than I expected. However, it resulted in an open door to fix
>>> some other PRs and the attached much larger patch.
>>>
>>> This time, I did remember to include the testcases in the .diff :-)
>>
>> indeed! :-)
>>
>> I've only had a superficial look so far although it looks very good.
>> (I have to trust your experience with unlimited polymorphism.)
>>
>> However, I was wondering about the following helper function:
>>
>> +bool
>> +gfc_is_ptr_fcn (gfc_expr *e)
>> +{
>> +  return e != NULL && e->expr_type == EXPR_FUNCTION
>> +             && (gfc_expr_attr (e).pointer
>> +                 || (e->ts.type == BT_CLASS
>> +                     && CLASS_DATA (e)->attr.class_pointer));
>> +}
>> +
>> +
>>    /* Copy a shape array.  */
>>
>> Is there a case where gfc_expr_attr (e).pointer returns false
>> and you really need the || part?  Looking at gfc_expr_attr
>> and the present context, it might just not be necessary.
>>
>>> I believe that, between the Change.Logs and the comments, it is
>>> reasonably self-explanatory.
>>>
>>> OK for trunk?
>>
>> OK from my side.
>>
>> Thanks for the patch!
>>
>> Harald
>>
>>> Regards
>>>
>>> Paul
>>>
>>> Fortran: Fix some bugs in associate [PR87477]
>>>
>>> 2023-06-20  Paul Thomas  <pault@gcc.gnu.org>
>>>
>>> gcc/fortran
>>> PR fortran/87477
>>> PR fortran/88688
>>> PR fortran/94380
>>> PR fortran/107900
>>> PR fortran/110224
>>> * decl.cc (char_len_param_value): Fix memory leak.
>>> (resolve_block_construct): Remove unnecessary static decls.
>>> * expr.cc (gfc_is_ptr_fcn): New function.
>>> (gfc_check_vardef_context): Use it to permit pointer function
>>> result selectors to be used for associate names in variable
>>> definition context.
>>> * gfortran.h: Prototype for gfc_is_ptr_fcn.
>>> * match.cc (build_associate_name): New function.
>>> (gfc_match_select_type): Use the new function to replace inline
>>> version and to build a new associate name for the case where
>>> the supplied associate name is already used for that purpose.
>>> * resolve.cc (resolve_assoc_var): Call gfc_is_ptr_fcn to allow
>>> associate names with pointer function targets to be used in
>>> variable definition context.
>>> * trans-decl.cc (gfc_get_symbol_decl): Unlimited polymorphic
>>> variables need deferred initialisation of the vptr.
>>> (gfc_trans_deferred_vars): Do the vptr initialisation.
>>> * trans-stmt.cc (trans_associate_var): Ensure that a pointer
>>> associate name points to the target of the selector and not
>>> the selector itself.
>>>
>>> gcc/testsuite/
>>> PR fortran/87477
>>> PR fortran/107900
>>> * gfortran.dg/pr107900.f90 : New test
>>>
>>> PR fortran/110224
>>> * gfortran.dg/pr110224.f90 : New test
>>>
>>> PR fortran/88688
>>> * gfortran.dg/pr88688.f90 : New test
>>>
>>> PR fortran/94380
>>> * gfortran.dg/pr94380.f90 : New test
>>>
>>> PR fortran/95398
>>> * gfortran.dg/pr95398.f90 : Set -std=f2008, bump the line
>>> numbers in the error tests by two and change the text in two.
>>
> 
> 



  parent reply	other threads:[~2023-06-21 18:40 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-17  9:14 Paul Richard Thomas
2023-06-17 15:33 ` Steve Kargl
2023-06-17 18:01 ` Harald Anlauf
2023-06-17 18:01   ` Harald Anlauf
     [not found]   ` <CAGkQGi+A5OuESANYKB=SOv1a4VqogCinV5+YCijn3+y+Pbq+mA@mail.gmail.com>
2023-06-20 10:54     ` Paul Richard Thomas
2023-06-20 21:57       ` Harald Anlauf
2023-06-20 21:57         ` Harald Anlauf
2023-06-21 16:12         ` Paul Richard Thomas
2023-06-21 16:46           ` Steve Kargl
2023-06-21 18:40           ` Harald Anlauf [this message]
2023-06-21 18:40             ` Harald Anlauf
2023-06-21 19:17       ` Bernhard Reutner-Fischer
2023-06-22  6:19         ` Paul Richard Thomas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=af7bfd03-3e6c-e869-b1c6-7d89bbdc9732@gmx.de \
    --to=anlauf@gmx.de \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=paul.richard.thomas@gmail.com \
    --cc=sgk@troutmask.apl.washington.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).