Re: [Patch, fortran PR89645/99065 No IMPLICIT type error with: ASSOCIATE( X => function() )

[Harald Anlauf <anlauf@gmx.de>]

Hi Paul,

welcome back!

On 3/3/24 17:04, Paul Richard Thomas wrote:
> Hi Harald,
> 
> Please find an updated version of the patch that rolls in Steve's patch for
> PR114141, fixes unlimited polymorphic function selectors and cures the
> memory leaks. I apologise for not working on this sooner but, as I informed
> you, I have been away for an extended trip to Australia.
> 
> The chunks that fix PR114141 are picked out in comment 14 to the PR and the
> cures to the problems that you found in the first review are found at
> trans-stmt.cc:2047-49.
> 
> Regtests fine. OK for trunk, bearing in mind that most of the patch is ring
> fenced by the inferred_type flag?

I would say that it is almost fine.

Two things that I found:

- Testcase associate_65.f90 does not compile with -std=f2023, because
   IMAG is a GNU extension, while AIMAG is the standard version.
   Could you please adjust that?

- I think the handling of parentheses and functions returning pointers
   does not work correctly.  Consider:


program paul
   implicit none
   type t
      integer :: i
   end type t
   type(t), pointer :: p(:)
   allocate (p(-3:3))

   associate (q => p)
     print *, lbound (q), ubound (q) ! Should print -3 3 (OK)
   end associate

   associate (q => set_ptr())
     print *, lbound (q), ubound (q) ! Should print -3 3 (OK)
   end associate

   associate (q => (p))
     print *, lbound (q), ubound (q) ! Should print 1 7 (OK)
   end associate

   associate (q => (set_ptr()))      ! <- are these parentheses lost?
     print *, lbound (q), ubound (q) ! Should print 1 7
   end associate
contains
   function set_ptr () result (res)
     type(t), pointer :: res(:)
     res => p
   end function set_ptr
end


While the first three variants give the right bounds, the last version
- after applying your patch - is mishandled and the testcase now prints:

           -3           3
           -3           3
            1           7
           -3           3

Both NAG and Intel support my expectation, namely that the last line
should equal the next-to-last.

Can you recheck the logic for that particular corner case?

With these points addressed, your patch is OK from my side.

Thanks for the patch and your endurance!

Harald


> Cheers
> 
> Paul
> 
> 
> On Mon, 8 Jan 2024 at 21:53, Harald Anlauf <anlauf@gmx.de> wrote:
> 
>> 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
>> ...snip...
>