Re: [Fortran, Patch, PR119272, v1] Fix handling of class' component call in associate

2025-03-19 Thread Andre Vehreschild
Hi Harald and Paul,

thanks for the reviews. Committed as gcc-15-8297-g9a13dc48a3a.

Paul, I am working on the change team construct at the moment. It has an
association list embedded. I will finish that work hopefully soon. After that
it will be safe for you to "get out your notes and restart" w/o producing to
many merge conflicts.

Thanks again and regards,
Andre

On Wed, 19 Mar 2025 10:51:49 +
Paul Richard Thomas  wrote:

> Hi Andre and Harald,
>
> It looks good to me too.
>
> Indeed, the associate construct is a strange one since TKR guessing is done
> at a very early stage so that the associate block can be parsed. About a
> year ago, I started looking at tackling this by delaying parsing the blocks
> until the containing block had been parsed and resolved. It nearly worked
> and I think that I should get out my notes and restart :-)
>
> In the meantime, this is more than band-aid, it is a necessary correction,
> given the way associate is parsed.
>
> Regards and thanks for the patch
>
> Paul
>
>
> On Tue, 18 Mar 2025 at 22:08, Harald Anlauf  wrote:
>
> > Hi Andre,
> >
> > Am 17.03.25 um 09:56 schrieb Andre Vehreschild:
> > > The issue is that the tbp (the typebound proc info structure) is not
> > resolved
> > > completely when the associate tries to do an early resolve to determine
> > the
> > > rank of the associate variable. When the expression to be resolved for
> > that
> > > contains a compcall, the resolve branches into the incorrect case and
> > emits the
> > > error. My current fix is to wait with generating the error message until
> > the
> > > type has been resolved completely (aka. symbol's resolve_symbol_called
> > is set).
> > > I am not sure, if this is correct, therefore CC'ing Paul, who, to my
> > > knowledge, has more experience in the associate area. But everyone
> > please feel
> > > free to step in!
> >
> > your solution looks basically correct to me, but I wonder why to
> > return early w/o error.  Would the following logic be wrong?
> >
> > @@ -7349,7 +7357,8 @@ resolve_compcall (gfc_expr* e, const char **name)
> > gfc_symtree* target;
> >
> > /* Check that's really a FUNCTION.  */
> > -  if (!e->value.compcall.tbp->function)
> > +  if (!e->value.compcall.tbp->function
> > +  && e->symtree && e->symtree->n.sym->resolve_symbol_called)
> >   {
> > gfc_error ("%qs at %L should be a FUNCTION",
> >   e->value.compcall.name, &e->where);
> >
> > Sorry if this is a stupid question.  And not regtested, although
> > it also fixes the original report.
> >
> > > Regtests ok on x86_64-pc-linux-gnu / F41. Ok for mainline?
> >
> > If neither Paul steps in nor anybody else, go ahead and commit.
> > Even if your patch were a band-aid, it does not look wrong, and
> > if it is later found to be it can be improved...
> >
> > Thanks,
> > Harald
> >
> > > Regards,
> > >   Andre
> > > --
> > > Andre Vehreschild * Email: vehre ad gmx dot de
> >
> >


--
Andre Vehreschild * Email: vehre ad gmx dot de


GSOC 2025 - Fortran - 2018/202x

2025-03-19 Thread Yuao Ma
Hello GCC Community,

My name is Yuao, and I’m excited to express my interest in the “Fortran – 
2018/202x” project for Google Summer of Code. I am majoring in Cyber Science 
and Engineering, with a solid background in computer science that I believe 
will help me contribute effectively to this project.

I am passionate about open-source software and have made contributions to 
several projects, including vcpkg and LLVM. Within LLVM, I’ve worked on Clang 
(the front-end), middle-end optimization passes, and llvm-libc. These 
experiences have allowed me to develop a strong understanding of 
production-grade compilers—expertise I hope to bring to the GCC Fortran project.

I would greatly appreciate any guidance or suggestions on how to get started, 
such as relevant documentation, communication channels, or initial tasks. Thank 
you for your time, and I look forward to the possibility of working with you 
and learning from the GCC community.

Kind regards, Yuao


Re: Bug 119349 [15 Regression] polymorphic array dummy argument with function result actual argument in elemental function

2025-03-19 Thread Jerry D

On 3/17/25 10:40 PM, Damian Rouson wrote:

With gfortran 12.4.0, 13.3.0, and 14.2.0, the program below prints
"T".   With gfortran 15, the program crashes with an error message
that indicates that a pointer is being freed that was not allocated.

% cat all.f90
   implicit none

   type string_t
 character(len=:), allocatable :: string_
   end type

   print *, true([string()])

contains

   type(string_t) function string()
 string%string_ = ""
   end function

   logical elemental function true(rhs)
 class(string_t), intent(in) :: rhs
 true = .true.
   end function

end

% gfortran all.f90
% ./a.out

a.out(28435,0x209bdc840) malloc: *** error for object 0x62f98060:
pointer being freed was not allocated
a.out(28435,0x209bdc840) malloc: *** set a breakpoint in
malloc_error_break to debug

Program received signal SIGABRT: Process abort signal.

Backtrace for this error:
#0  0x1028d2563
#1  0x1028d14e3
#2  0x1a02a2de3
#3  0x1a026bf6f
#4  0x1a0178907
#5  0x1a0081e37
#6  0x1a00859bb
#7  0x1a00a4143
#8  0x102583bdf
#9  0x102583daf
zsh: abort  ./a.out

% gfortran --version
GNU Fortran (GCC) 15.0.1 20250315 (experimental)


After looking at the -fdummp-tree-original I am sure this is out of my 
area. It is certainly a recent regression.


Jerry


[PATCH] Fortran: fix bogus bounds check for reallocation on assignment [PR116706]

2025-03-19 Thread Harald Anlauf

Dear all,

the attached patch addresses an actually very long-standing issue
with bogus bounds checks for components of nested derived types in
assignments when an intermediate level has the POINTER attribute
instead of the ALLOCATABLE attribute.  It turned out that the
check for a reallocatable lhs failed in such situations, as it
depended on an attribute that is currently not properly set.

I did not see a way to fix the alloc_comp attribute so that
it can deal with the current situation and decided to remove
that check in gfc_is_reallocatable_lhs.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

As this issue leads to wrong code, is it OK to backport
e.g. to 14-branch?

Thanks,
Harald

From 74ef401638194bfc86fec3e78b451445ed86eabe Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Wed, 19 Mar 2025 22:56:03 +0100
Subject: [PATCH] Fortran: fix bogus bounds check for reallocation on
 assignment [PR116706]

	PR fortran/116706

gcc/fortran/ChangeLog:

	* trans-array.cc (gfc_is_reallocatable_lhs): Fix check on
	allocatable components of derived type or class objects.

gcc/testsuite/ChangeLog:

	* gfortran.dg/bounds_check_27.f90: New test.
---
 gcc/fortran/trans-array.cc|  4 +-
 gcc/testsuite/gfortran.dg/bounds_check_27.f90 | 45 +++
 2 files changed, 46 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_27.f90

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index 8ab290bbe61..e9eacf20128 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -11236,9 +11236,7 @@ gfc_is_reallocatable_lhs (gfc_expr *expr)
 return true;
 
   /* All that can be left are allocatable components.  */
-  if ((sym->ts.type != BT_DERIVED
-   && sym->ts.type != BT_CLASS)
-	|| !sym->ts.u.derived->attr.alloc_comp)
+  if (sym->ts.type != BT_DERIVED && sym->ts.type != BT_CLASS)
 return false;
 
   /* Find a component ref followed by an array reference.  */
diff --git a/gcc/testsuite/gfortran.dg/bounds_check_27.f90 b/gcc/testsuite/gfortran.dg/bounds_check_27.f90
new file mode 100644
index 000..678aef63af6
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/bounds_check_27.f90
@@ -0,0 +1,45 @@
+! { dg-do run }
+! { dg-additional-options "-fcheck=bounds" }
+!
+! PR fortran/116706 - bogus bounds check for reallocation on assignment
+! Contributed by Balint Aradi  
+
+program testprog
+  implicit none
+
+  type :: data_node
+ integer, allocatable :: data(:)
+  end type data_node
+
+  type :: data_list
+ type(data_node), pointer :: nodes(:) => null()
+  end type data_list
+
+  type :: upoly_node
+ class(*), allocatable :: data(:)
+  end type upoly_node
+
+  type :: star_list
+ type(upoly_node), pointer :: nodes(:) => null()
+  end type star_list
+
+  type(data_list) :: datalist
+  type(star_list) :: starlist
+  class(star_list), allocatable :: astarlist
+  class(star_list), pointer :: pstarlist
+
+  allocate (datalist%nodes(2))
+  datalist%nodes(1)%data = [1, 2, 3]
+
+  allocate (starlist%nodes(2))
+  starlist%nodes(1)%data = [1., 2., 3.]
+
+  allocate (astarlist)
+  allocate (astarlist%nodes(2))
+  astarlist%nodes(1)%data = [1, 2, 3]
+
+  allocate (pstarlist)
+  allocate (pstarlist%nodes(2))
+  pstarlist%nodes(1)%data = [1., 2., 3.]
+
+end program testprog
-- 
2.43.0



Re: [PATCH] Fortran: fix bogus bounds check for reallocation on assignment [PR116706]

2025-03-19 Thread Harald Anlauf

Am 19.03.25 um 23:25 schrieb Steve Kargl:

On Wed, Mar 19, 2025 at 11:08:58PM +0100, Harald Anlauf wrote:


the attached patch addresses an actually very long-standing issue
with bogus bounds checks for components of nested derived types in
assignments when an intermediate level has the POINTER attribute
instead of the ALLOCATABLE attribute.  It turned out that the
check for a reallocatable lhs failed in such situations, as it
depended on an attribute that is currently not properly set.

I did not see a way to fix the alloc_comp attribute so that
it can deal with the current situation and decided to remove
that check in gfc_is_reallocatable_lhs.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?


OK.


As this issue leads to wrong code, is it OK to backport
e.g. to 14-branch?


Yes, fixing wrong-code bugs can/should be backported.


Alright, pushed as r15-8453-g3292ca9b0818c3 so far.


One question below.


+program testprog
+  implicit none
+
+  type :: data_node
+ integer, allocatable :: data(:)
+  end type data_node


data is of type integer.


+  allocate (starlist%nodes(2))
+  starlist%nodes(1)%data = [1., 2., 3.]


Is the floating-point conversion testing some aspect
of the bugfix that I don't see?


There's no conversion happening for a class(*) lhs,
which I have also checked (manually).  I could have used
character or logical in the rhs constructor instead...
Not important, though.

Thanks for the review!

Harald



Bug 119380 - [12,13,14,15] Associate malloc error on selector with allocatable and procedure pointer

2025-03-19 Thread Damian Rouson
An associate construct causes the alloc error "pointer being freed was
not allocated" when the selector is a structure constructor for a
derived type with an allocatable component and a procedure pointer
component.  This same error occurs if a function returning the same
type replaces the structure constructor. The same error occurs for
programs compiled with gfortran 12.4.0, 13.3.0, 14.2.0, and 15.0.1:

% cat all.f90
  implicit none

  type foo_t
integer, allocatable :: i_
procedure(f), pointer, nopass :: f_
  end type

  associate(foo => foo_t(1,f))
  end associate

contains

  function f()
logical, allocatable :: f
f = .true.
  end function

end

% gfortran all.f90
% ./a.out
a.out(99502,0x1ed7e4840) malloc: *** error for object 0x2:
pointer being freed was not allocated
a.out(99502,0x1ed7e4840) malloc: *** set a breakpoint in
malloc_error_break to debug

Program received signal SIGABRT: Process abort signal.

Backtrace for this error:
#0  0x100e7a563
#1  0x100e794e3
#2  0x183eaade3
#3  0x183e73f6f
#4  0x183d80907
#5  0x183c89e37
#6  0x183c8d9bb
#7  0x183cac143
#8  0x100acfa43
#9  0x100acfabf
zsh: abort  ./a.out

% gfortran --version
GNU Fortran (GCC) 15.0.1 20250315 (experimental)


Re: [PATCH] Fortran: fix bogus bounds check for reallocation on assignment [PR116706]

2025-03-19 Thread Steve Kargl
On Wed, Mar 19, 2025 at 11:08:58PM +0100, Harald Anlauf wrote:
> 
> the attached patch addresses an actually very long-standing issue
> with bogus bounds checks for components of nested derived types in
> assignments when an intermediate level has the POINTER attribute
> instead of the ALLOCATABLE attribute.  It turned out that the
> check for a reallocatable lhs failed in such situations, as it
> depended on an attribute that is currently not properly set.
> 
> I did not see a way to fix the alloc_comp attribute so that
> it can deal with the current situation and decided to remove
> that check in gfc_is_reallocatable_lhs.
> 
> Regtested on x86_64-pc-linux-gnu.  OK for mainline?

OK.

> As this issue leads to wrong code, is it OK to backport
> e.g. to 14-branch?

Yes, fixing wrong-code bugs can/should be backported.

One question below.

> +program testprog
> +  implicit none
> +
> +  type :: data_node
> + integer, allocatable :: data(:)
> +  end type data_node

data is of type integer.

> +  allocate (starlist%nodes(2))
> +  starlist%nodes(1)%data = [1., 2., 3.]

Is the floating-point conversion testing some aspect 
of the bugfix that I don't see?


-- 
Steve


Re: [Fortran, Patch, PR119272, v1] Fix handling of class' component call in associate

2025-03-19 Thread Paul Richard Thomas
Hi Andre and Harald,

It looks good to me too.

Indeed, the associate construct is a strange one since TKR guessing is done
at a very early stage so that the associate block can be parsed. About a
year ago, I started looking at tackling this by delaying parsing the blocks
until the containing block had been parsed and resolved. It nearly worked
and I think that I should get out my notes and restart :-)

In the meantime, this is more than band-aid, it is a necessary correction,
given the way associate is parsed.

Regards and thanks for the patch

Paul


On Tue, 18 Mar 2025 at 22:08, Harald Anlauf  wrote:

> Hi Andre,
>
> Am 17.03.25 um 09:56 schrieb Andre Vehreschild:
> > The issue is that the tbp (the typebound proc info structure) is not
> resolved
> > completely when the associate tries to do an early resolve to determine
> the
> > rank of the associate variable. When the expression to be resolved for
> that
> > contains a compcall, the resolve branches into the incorrect case and
> emits the
> > error. My current fix is to wait with generating the error message until
> the
> > type has been resolved completely (aka. symbol's resolve_symbol_called
> is set).
> > I am not sure, if this is correct, therefore CC'ing Paul, who, to my
> > knowledge, has more experience in the associate area. But everyone
> please feel
> > free to step in!
>
> your solution looks basically correct to me, but I wonder why to
> return early w/o error.  Would the following logic be wrong?
>
> @@ -7349,7 +7357,8 @@ resolve_compcall (gfc_expr* e, const char **name)
> gfc_symtree* target;
>
> /* Check that's really a FUNCTION.  */
> -  if (!e->value.compcall.tbp->function)
> +  if (!e->value.compcall.tbp->function
> +  && e->symtree && e->symtree->n.sym->resolve_symbol_called)
>   {
> gfc_error ("%qs at %L should be a FUNCTION",
>   e->value.compcall.name, &e->where);
>
> Sorry if this is a stupid question.  And not regtested, although
> it also fixes the original report.
>
> > Regtests ok on x86_64-pc-linux-gnu / F41. Ok for mainline?
>
> If neither Paul steps in nor anybody else, go ahead and commit.
> Even if your patch were a band-aid, it does not look wrong, and
> if it is later found to be it can be improved...
>
> Thanks,
> Harald
>
> > Regards,
> >   Andre
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
>
>


Re: [Fortran, Patch, PR119272, v1] Fix handling of class' component call in associate

2025-03-19 Thread Andre Vehreschild
Hi Harald,

thanks for the comments.

> your solution looks basically correct to me, but I wonder why to
> return early w/o error.  Would the following logic be wrong?
>
> @@ -7349,7 +7357,8 @@ resolve_compcall (gfc_expr* e, const char **name)
> gfc_symtree* target;
>
> /* Check that's really a FUNCTION.  */
> -  if (!e->value.compcall.tbp->function)
> +  if (!e->value.compcall.tbp->function
> +  && e->symtree && e->symtree->n.sym->resolve_symbol_called)
>   {
> gfc_error ("%qs at %L should be a FUNCTION",
>   e->value.compcall.name, &e->where);

I didn't see the implications on first glance either, but when I combine the
two if()s, then I get four times the wrong error

Error: 'my_sub' at (1) is not a function

in typebound_call_25.f90 from the testsuite. Please note, that my initial patch
only prevents generating the error, but return false in any case. Combining the
two if()s would also prevent the return false (or I interpreted your questions
wrong). This gives different behavior. So I am curious about what Paul's
opinion is, but nevertheless feel free to voice up, if I did something wrong or
mis-unterstood your intention.

Thanks for looking at this and

Regards,
Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de


Re: [Patch, fortran] PR85836: Implement the F2018 reduce intrinsic

2025-03-19 Thread Paul Richard Thomas
Hi Andre,

Thanks for the review - I'll act on the points that you raised.

The Linaro people reported a failure in reduce_1.f90 execution, which I
believe is due to incorrect casting of 'dim' and a wrong specification of
its kind. I am waiting to hear back from them as to whether or not I have
fixed the failure.

Cheers

Paul


On Wed, 19 Mar 2025 at 12:39, Andre Vehreschild  wrote:

> Hi Paul,
>
> I took a look at your patch and think I found some improvements needed. In
>
> +bool
> +gfc_check_reduce (gfc_expr *array, gfc_expr *operation, gfc_expr *dim,
> + gfc_expr *mask, gfc_expr *identity, gfc_expr *ordered)
> +{
>
> ...
>
> +  if (formal->sym->attr.allocatable || formal->sym->attr.allocatable
> +  || formal->sym->attr.pointer || formal->sym->attr.pointer
> +  || formal->sym->attr.optional || formal->sym->attr.optional
> +  || formal->sym->ts.type == BT_CLASS || formal->sym->ts.type ==
> BT_CLASS)
> +{
> +  gfc_error ("Each argument of OPERATION at %L shall be a scalar, "
> +"non-allocatable, non-pointer, non-polymorphic and "
> +"nonoptional", &operation->where);
> +  return false;
> +}
>
> The if is only looking at the first formal argument. The right-hand sides
> of the || miss a ->next-> to look at the second formal argument, right?
>
> May be you also want to extend the tests!?
>
> Without having looked at it, but can't you extract the whole block of
>
> +  if (array->ts.type == BT_CHARACTER)
> +{
> +  unsigned long actual_size, formal_size1, formal_size2, result_size;
> ...
> + return false;
> +   }
> +}
>
> and share it with the checks for co_reduce? I figure way to many DRY
> principle
> violations are in gfortran. So when we can start this, why not do it? And a
> call to a routine, like check_char_arg_conformance() speaks way better,
> then
> having to read all that code ;-)
>
> In gfc_resolve_reduce() identity and ordered are marked as UNUSED. Should
> these
> not a least be resolved?
>
> Please run contrib/check_GNU_style on your patch. It reports several issues
> (haven't look into their validity).
>
> In the Changelog:
>
> -   (gfc_check_rename): Add prototype for intrinsic with 6 arguments.
> +   * gfortran.h: Add prototype for intrinsic with 6 arguments.
>
> s/discription/description/
>
> I also encountered that nit with the executable stack when working in
> OpenCoarrays, but haven't had time (or desire) to look into it. I will put
> myself into CC of the pr Jerry mentioned.
>
> Besides the mentions above, this looks good to me.
>
> Thanks for the patch and
>
> Regards,
> Andre
>
>
>
> On Sun, 16 Mar 2025 17:26:55 +
> Paul Richard Thomas  wrote:
>
> > Hi All,
> >
> > This version of the REDUCE intrinsic patch has evolved somewhat since the
> > posting on 2nd March. The most important changes are to the wrapper
> > function and the addition of two testsuite entries.
> >
> > The wrapper function now effects:
> > subroutine wrapper (a, b, c)
> >  type_of_ARRAY, intent(inout) :: a, c
> >  type_of_ARRAY, intent(inout), optional :: b
> >  if (present (b)) then
> > c = OPERATION (a,b )
> >  else
> > c = a
> >  end if
> > end subroutine
> >
> > The reason for wrapping OPERATION in a subroutine is to allow pointer
> > arithmetic to be used throughout in the library function. The only thing
> > that needs to be known about the type and kind of ARRAY is the element
> > size. The second branch in the wrapper allows deep copies to be done in
> the
> > library function, such that derived types with allocatable components do
> > not leak memory. This is needed at the final step of the algorithm to
> copy
> > the result from each iteration to the result and then nullify it.
> >
> > This is undoubtedly a bit heavy going for intrinsic types and so, one day
> > soon I will possibly do a bit of M4ery. That said, the present version
> > works for all types of ARRAY and I worry a bit about how much this
> > intrinsic will be used. Thoughts?
> >
> > A slight niggle is the linker error that comes up if compiled without any
> > optimization:
> > /usr/bin/ld: warning: /tmp/cc9cx9Rw.o: requires executable stack (because
> > the .note.GNU-stack section is executable)
> > I think that this is unlikely to present a security issue, however, since
> > it disappears at -O1, I went through each of the options triggered by -O1
> > but couldn't make it go away. Does anybody know why this is?
> >
> > Regtests OK with FC41/x86_64 - OK for mainline?
> >
> > Regards
> >
> > Paul
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de
>


Re: [Fortran, Patch, PR119272, v1] Fix handling of class' component call in associate

2025-03-19 Thread Paul Richard Thomas
Hi Andre,

Do not worry about the change team construct. Getting out my notes and
restarting is likely to take a few months yet. Besides which, the PDT fixes
have to come first.

Cheers

Paul


On Wed, 19 Mar 2025 at 13:59, Andre Vehreschild  wrote:

> Hi Harald and Paul,
>
> thanks for the reviews. Committed as gcc-15-8297-g9a13dc48a3a.
>
> Paul, I am working on the change team construct at the moment. It has an
> association list embedded. I will finish that work hopefully soon. After
> that
> it will be safe for you to "get out your notes and restart" w/o producing
> to
> many merge conflicts.
>
> Thanks again and regards,
> Andre
>
> On Wed, 19 Mar 2025 10:51:49 +
> Paul Richard Thomas  wrote:
>
> > Hi Andre and Harald,
> >
> > It looks good to me too.
> >
> > Indeed, the associate construct is a strange one since TKR guessing is
> done
> > at a very early stage so that the associate block can be parsed. About a
> > year ago, I started looking at tackling this by delaying parsing the
> blocks
> > until the containing block had been parsed and resolved. It nearly worked
> > and I think that I should get out my notes and restart :-)
> >
> > In the meantime, this is more than band-aid, it is a necessary
> correction,
> > given the way associate is parsed.
> >
> > Regards and thanks for the patch
> >
> > Paul
> >
> >
> > On Tue, 18 Mar 2025 at 22:08, Harald Anlauf  wrote:
> >
> > > Hi Andre,
> > >
> > > Am 17.03.25 um 09:56 schrieb Andre Vehreschild:
> > > > The issue is that the tbp (the typebound proc info structure) is not
> > > resolved
> > > > completely when the associate tries to do an early resolve to
> determine
> > > the
> > > > rank of the associate variable. When the expression to be resolved
> for
> > > that
> > > > contains a compcall, the resolve branches into the incorrect case and
> > > emits the
> > > > error. My current fix is to wait with generating the error message
> until
> > > the
> > > > type has been resolved completely (aka. symbol's
> resolve_symbol_called
> > > is set).
> > > > I am not sure, if this is correct, therefore CC'ing Paul, who, to my
> > > > knowledge, has more experience in the associate area. But everyone
> > > please feel
> > > > free to step in!
> > >
> > > your solution looks basically correct to me, but I wonder why to
> > > return early w/o error.  Would the following logic be wrong?
> > >
> > > @@ -7349,7 +7357,8 @@ resolve_compcall (gfc_expr* e, const char **name)
> > > gfc_symtree* target;
> > >
> > > /* Check that's really a FUNCTION.  */
> > > -  if (!e->value.compcall.tbp->function)
> > > +  if (!e->value.compcall.tbp->function
> > > +  && e->symtree && e->symtree->n.sym->resolve_symbol_called)
> > >   {
> > > gfc_error ("%qs at %L should be a FUNCTION",
> > >   e->value.compcall.name, &e->where);
> > >
> > > Sorry if this is a stupid question.  And not regtested, although
> > > it also fixes the original report.
> > >
> > > > Regtests ok on x86_64-pc-linux-gnu / F41. Ok for mainline?
> > >
> > > If neither Paul steps in nor anybody else, go ahead and commit.
> > > Even if your patch were a band-aid, it does not look wrong, and
> > > if it is later found to be it can be improved...
> > >
> > > Thanks,
> > > Harald
> > >
> > > > Regards,
> > > >   Andre
> > > > --
> > > > Andre Vehreschild * Email: vehre ad gmx dot de
> > >
> > >
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de
>


Re: [Patch, fortran] PR85836: Implement the F2018 reduce intrinsic

2025-03-19 Thread Andre Vehreschild
Hi Paul,

I took a look at your patch and think I found some improvements needed. In

+bool
+gfc_check_reduce (gfc_expr *array, gfc_expr *operation, gfc_expr *dim,
+ gfc_expr *mask, gfc_expr *identity, gfc_expr *ordered)
+{

...

+  if (formal->sym->attr.allocatable || formal->sym->attr.allocatable
+  || formal->sym->attr.pointer || formal->sym->attr.pointer
+  || formal->sym->attr.optional || formal->sym->attr.optional
+  || formal->sym->ts.type == BT_CLASS || formal->sym->ts.type == BT_CLASS)
+{
+  gfc_error ("Each argument of OPERATION at %L shall be a scalar, "
+"non-allocatable, non-pointer, non-polymorphic and "
+"nonoptional", &operation->where);
+  return false;
+}

The if is only looking at the first formal argument. The right-hand sides
of the || miss a ->next-> to look at the second formal argument, right?

May be you also want to extend the tests!?

Without having looked at it, but can't you extract the whole block of

+  if (array->ts.type == BT_CHARACTER)
+{
+  unsigned long actual_size, formal_size1, formal_size2, result_size;
...
+ return false;
+   }
+}

and share it with the checks for co_reduce? I figure way to many DRY principle
violations are in gfortran. So when we can start this, why not do it? And a
call to a routine, like check_char_arg_conformance() speaks way better, then
having to read all that code ;-)

In gfc_resolve_reduce() identity and ordered are marked as UNUSED. Should these
not a least be resolved?

Please run contrib/check_GNU_style on your patch. It reports several issues
(haven't look into their validity).

In the Changelog:

-   (gfc_check_rename): Add prototype for intrinsic with 6 arguments.
+   * gfortran.h: Add prototype for intrinsic with 6 arguments.

s/discription/description/

I also encountered that nit with the executable stack when working in
OpenCoarrays, but haven't had time (or desire) to look into it. I will put
myself into CC of the pr Jerry mentioned.

Besides the mentions above, this looks good to me.

Thanks for the patch and

Regards,
Andre



On Sun, 16 Mar 2025 17:26:55 +
Paul Richard Thomas  wrote:

> Hi All,
>
> This version of the REDUCE intrinsic patch has evolved somewhat since the
> posting on 2nd March. The most important changes are to the wrapper
> function and the addition of two testsuite entries.
>
> The wrapper function now effects:
> subroutine wrapper (a, b, c)
>  type_of_ARRAY, intent(inout) :: a, c
>  type_of_ARRAY, intent(inout), optional :: b
>  if (present (b)) then
> c = OPERATION (a,b )
>  else
> c = a
>  end if
> end subroutine
>
> The reason for wrapping OPERATION in a subroutine is to allow pointer
> arithmetic to be used throughout in the library function. The only thing
> that needs to be known about the type and kind of ARRAY is the element
> size. The second branch in the wrapper allows deep copies to be done in the
> library function, such that derived types with allocatable components do
> not leak memory. This is needed at the final step of the algorithm to copy
> the result from each iteration to the result and then nullify it.
>
> This is undoubtedly a bit heavy going for intrinsic types and so, one day
> soon I will possibly do a bit of M4ery. That said, the present version
> works for all types of ARRAY and I worry a bit about how much this
> intrinsic will be used. Thoughts?
>
> A slight niggle is the linker error that comes up if compiled without any
> optimization:
> /usr/bin/ld: warning: /tmp/cc9cx9Rw.o: requires executable stack (because
> the .note.GNU-stack section is executable)
> I think that this is unlikely to present a security issue, however, since
> it disappears at -O1, I went through each of the options triggered by -O1
> but couldn't make it go away. Does anybody know why this is?
>
> Regtests OK with FC41/x86_64 - OK for mainline?
>
> Regards
>
> Paul


--
Andre Vehreschild * Email: vehre ad gmx dot de