public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Harald Anlauf <anlauf@gmx.de>
To: Paul Richard Thomas <paul.richard.thomas@gmail.com>,
	"fortran@gcc.gnu.org" <fortran@gcc.gnu.org>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [Patch, fortran PR89645/99065 No IMPLICIT type error with: ASSOCIATE( X => function() )
Date: Mon, 8 Jan 2024 22:53:21 +0100	[thread overview]
Message-ID: <674cfbf0-a95d-4433-a025-29e78dc0bcef@gmx.de> (raw)
In-Reply-To: <CAGkQGiLh4RyAki7FV8vVLJDZydqT3hcaLKyMFjt4qvkEmWQiLg@mail.gmail.com>

Hi Paul,

your patch looks already very impressive!

Regarding the patch as is, I am still trying to grok it, even with your
explanations at hand...

While the testcase works as advertised, I noticed that it exhibits a
runtime memleak that occurs for (likely) each case where the associate
target is an allocatable, class-valued function result.

I tried to produce a minimal testcase using class(*), which apparently
is not handled by your patch (it ICEs for me):

program p
   implicit none
   class(*), allocatable :: x(:)
   x = foo()
   call prt (x)
   deallocate (x)
   ! up to here no memleak...
   associate (var => foo())
     call prt (var)
   end associate
contains
   function foo() result(res)
     class(*), allocatable :: res(:)
     res = [42]
   end function foo
   subroutine prt (x)
     class(*), intent(in) :: x(:)
     select type (x)
     type is (integer)
        print *, x
     class default
        stop 99
     end select
   end subroutine prt
end

Traceback (truncated):

foo.f90:9:18:

     9 |     call prt (var)
       |                  1
internal compiler error: tree check: expected record_type or union_type
or qual_union_type, have function_type in gfc_class_len_get, at
fortran/trans-expr.cc:271
0x19fd5d5 tree_check_failed(tree_node const*, char const*, int, char
const*, ...)
         ../../gcc-trunk/gcc/tree.cc:8952
0xe1562d tree_check3(tree_node*, char const*, int, char const*,
tree_code, tree_code, tree_code)
         ../../gcc-trunk/gcc/tree.h:3652
0xe3e264 gfc_class_len_get(tree_node*)
         ../../gcc-trunk/gcc/fortran/trans-expr.cc:271
0xecda48 trans_associate_var
         ../../gcc-trunk/gcc/fortran/trans-stmt.cc:2325
0xecdd09 gfc_trans_block_construct(gfc_code*)
         ../../gcc-trunk/gcc/fortran/trans-stmt.cc:2383
[...]

I don't see anything wrong with it: NAG groks it, like Nvidia and Flang,
while Intel crashes at runtime.

Can you have another brief look?

Thanks,
Harald


On 1/6/24 18:26, Paul Richard Thomas wrote:
> These PRs come about because of gfortran's single pass parsing. If the
> function in the title is parsed after the associate construct, then its
> type and rank are not known. The point at which this becomes a problem is
> when expressions within the associate block are parsed. primary.cc
> (gfc_match_varspec) could already deal with intrinsic types and so
> component references were the trigger for the problem.
>
> The two major parts of this patch are the fixup needed in gfc_match_varspec
> and the resolution of  expressions with references in resolve.cc
> (gfc_fixup_inferred_type_refs). The former relies on the two new functions
> in symbol.cc to search for derived types with an appropriate component to
> match the component reference and then set the associate name to have a
> matching derived type. gfc_fixup_inferred_type_refs is called in resolution
> and so the type of the selector function is known.
> gfc_fixup_inferred_type_refs ensures that the component references use this
> derived type and that array references occur in the right place in
> expressions and match preceding array specs. Most of the work in preparing
> the patch was sorting out cases where the selector was not a derived type
> but, instead, a class function. If it were not for this, the patch would
> have been submitted six months ago :-(
>
> The patch is relatively safe because most of the chunks are guarded by
> testing for the associate name being an inferred type, which is set in
> gfc_match_varspec. For this reason, I do not think it likely that the patch
> will cause regressions. However, it is more than possible that variants not
> appearing in the submitted testcase will throw up new bugs.
>
> Jerry has already given the patch a whirl and found that it applies
> cleanly, regtests OK and works as advertised.
>
> OK for trunk?
>
> Paul
>
> Fortran: Fix class/derived type function associate selectors [PR87477]
>
> 2024-01-06  Paul Thomas  <pault@gcc.gnu.org>
>
> gcc/fortran
> PR fortran/87477
> PR fortran/89645
> PR fortran/99065
> * class.cc (gfc_change_class): New function needed for
> associate names, when rank changes or a derived type is
> produced by resolution
> * dump-parse-tree.cc (show_code_node): Make output for SELECT
> TYPE more comprehensible.
> * gfortran.h : Add 'gfc_association_list' to structure
> 'gfc_association_list'. Add prototypes for
> 'gfc_find_derived_types', 'gfc_fixup_inferred_type_refs' and
> 'gfc_change_class'. Add macro IS_INFERRED_TYPE.
> * match.cc (copy_ts_from_selector_to_associate): Add bolean arg
> 'select_type' with default false. If this is a select type name
> and the selector is a inferred type, build the class type and
> apply it to the associate name.
> (build_associate_name): Pass true to 'select_type' in call to
> previous.
> * parse.cc (parse_associate): If the selector is a inferred type
> the associate name is too. Make sure that function selector
> class and rank, if known, are passed to the associate name. If
> a function result exists, pass its typespec to the associate
> name.
> * primary.cc (gfc_match_varspec): If a scalar derived type
> select type temporary has an array reference, match the array
> reference, treating this in the same way as an equivalence
> member. If this is a inferred type with a component reference,
> call 'gfc_find_derived_types' to find a suitable derived type.
> * resolve.cc (resolve_variable): Call new function below.
> (gfc_fixup_inferred_type_refs): New function to ensure that the
> expression references for a inferred type are consistent with
> the now fixed up selector.
> (resolve_assoc_var): Ensure that derived type or class function
> selectors transmit the correct arrayspec to the associate name.
> (resolve_select_type): If the selector is an associate name of
> inferred type and has no component references, the associate
> name should have its typespec.
> * symbol.cc (gfc_set_default_type): If an associate name with
> unknown type has a selector expression, try resolving the expr.
> (find_derived_types, gfc_find_derived_types): New functions
> that search for a derived type with a given name.
> * trans-expr.cc (gfc_conv_variable): Some inferred type exprs
> escape resolution so call 'gfc_fixup_inferred_type_refs'.
> * trans-stmt.cc (trans_associate_var): Tidy up expression for
> 'class_target'. Correctly handle selectors that are class array
> references, passed as derived types.
>
> gcc/testsuite/
> PR fortran/87477
> PR fortran/89645
> PR fortran/99065
> * gfortran.dg/associate_64.f90 : New test
>


  reply	other threads:[~2024-01-08 21:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-06 17:26 Paul Richard Thomas
2024-01-08 21:53 ` Harald Anlauf [this message]
2024-03-03 16:04   ` Paul Richard Thomas
2024-03-03 20:20     ` Harald Anlauf
2024-03-12 14:54       ` Paul Richard Thomas
2024-03-12 21:07         ` Harald Anlauf
2024-03-12 21:28           ` 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=674cfbf0-a95d-4433-a025-29e78dc0bcef@gmx.de \
    --to=anlauf@gmx.de \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=paul.richard.thomas@gmail.com \
    /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).