Hi Harald,
>
> Reviewing it properly is beyond me, but if nobody else volunteers,
>
I am not so sure that is right but your review is mightily helpful. I worry
that the logic in the handling of the gfc_ss's is not clear enough. Some
time, when feeling bored, I might try to refactor it.
> I'll just provide a few minor comments derived from playing with it,
> and let you decide to push or polish.
>
> In testcase class_transformational_2.f90 I recommend to "harden"
> the select type () blocks in check_result and check_result_a by e.g.
>
> class default
> stop stop_flag + 4
>
> so that wrong dynamic types are detected (I had fun with ifx... :).
>
Done.
> There are also unused declared variables (e.g. ss(:)), probably left
> overs.
>
All unused variable warnings have been suppressed. The 'atmp' warnings were
removed at trans-array.cc:1632 by adding:
if (fcn_ss && fcn_ss->info && fcn_ss->info->class_container)
{
suppress_warning (desc);
TREE_USED (desc) = 0;
}
> I got an ICE compiling class_transformational_1.f90 with the following
> options:
>
> % gfc-15 class_transformational_1.f90 -O3 -fcheck=bounds
>
> Note that -O3 and -fcheck=bounds seem essential. I get:
>
> Indeed, I can reproduce the ICE.
> I did not try to narrow that down further. If you decide to push
> the submitted patch, and if you can reproduce the above, then please
> open a separate PR to track this. Given what the patch resolves,
> this is not a real show-stopper, so you may go ahead.
>
Committed as r15-5897. I will be following up with a new PR for the ICE.
Thanks for the review.
Paul
>
> Thanks,
> Harald
>
> Am 29.11.24 um 23:21 schrieb Paul Richard Thomas:
> > Hi Harald,
> >
> > Sorry about that - it was the standard HEAD versus HEAD~ mistake.
> >
> > Thanks for pointing it out.
> >
> > Paul
> >
> >
> > On Fri, 29 Nov 2024 at 17:31, Harald Anlauf <[email protected]> wrote:
> >
> >> Hi Paul,
> >>
> >> the patch seems to contain stuff that has already been pushed
> >> (gcc/testsuite/gfortran.dg/pr117768.f90, and the chunks in
> >> class.cc and resolve.cc). Can you please check?
> >>
> >> Cheers,
> >> Harald
> >>
> >> Am 29.11.24 um 17:34 schrieb Paul Richard Thomas:
> >>> Hi All,
> >>>
> >>> This patch was originally pushed as r15-2739. Subsequently memory
> faults
> >>> were found and so the patch was reverted. At the time, I could find
> where
> >>> the problem lay. This morning I had another look and found it almost
> >>> immediately :-)
> >>>
> >>> The fix is the 'gfc_resize_class_size_with_len' in the chunk '@@
> -1595,14
> >>> +1629,51 @@ gfc_trans_create_temp_array '. Without it,, half as much
> >> memory
> >>> as needed was being provided by the allocation and so accesses were
> >>> occurring outside the allocated space. Valgrind now reports no errors.
> >>>
> >>> Regression tests with flying colours - OK for mainline?
> >>>
> >>> Paul
> >>>
> >>
> >>
> >
>
>
