[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |anlauf at gcc dot gnu.org
   Last reconfirmed|2020-01-29 00:00:00         |2020-12-6
           Priority|P3                          |P4

--- Comment #2 from anlauf at gcc dot gnu.org ---
Reduced even further.

program p
  print *, +[ real :: +(1) ]
  print *, +[ real :: +[1] ]
end

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #3 from anlauf at gcc dot gnu.org ---
The case

program p
  print *, +[ real :: +(1) ]
end

is solved by e.g.

diff --git a/gcc/fortran/arith.c b/gcc/fortran/arith.c
index c4c1041afdf..b2fbeddeb49 100644
--- a/gcc/fortran/arith.c
+++ b/gcc/fortran/arith.c
@@ -1268,6 +1268,11 @@ reduce_unary (arith (*eval) (gfc_expr *, gfc_expr **),
gfc_expr *op,
   head = gfc_constructor_copy (op->value.constructor);
   for (c = gfc_constructor_first (head); c; c = gfc_constructor_next (c))
     {
+      if (c->expr->expr_type == EXPR_FUNCTION
+         && c->expr->value.function.isym
+         && c->expr->value.function.isym->elemental)
+       gfc_simplify_expr (c->expr, 1);
+
       rc = reduce_unary (eval, c->expr, &r);

       if (rc != ARITH_OK)


Another case of missed simplification?

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[gscfq@t-online.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #4 from G. Steinmetz <gscfq@t-online.de> ---

Case from c0 works now :

$ gfortran-13-20221009 z1.f90 && ./a.out
   1.00000000       2.00000000
   1.00000000       2.00000000

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=107216
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |anlauf at gcc dot gnu.org

--- Comment #5 from anlauf at gcc dot gnu.org ---
Submitted: https://gcc.gnu.org/pipermail/fortran/2022-October/058332.html

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kargl at gcc dot gnu.org

--- Comment #6 from kargl at gcc dot gnu.org ---
(In reply to anlauf from comment #5)
> Submitted: https://gcc.gnu.org/pipermail/fortran/2022-October/058332.html

Harald, I looked at your patch and agree that simplification should be done.
I don't know why I did not do it when I wrote walk_array_constructor().

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #7 from anlauf at gcc dot gnu.org ---
(In reply to kargl from comment #6)
> Harald, I looked at your patch and agree that simplification should be done.
> I don't know why I did not do it when I wrote walk_array_constructor().

Because you had only nice Fortran code in mind and for testing?

Well, I was wondering where the necessary simplifications would fit best.
walk_array_constructor is just one place, and if it hasn't happened before,
then it needs to be ensured there.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #8 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Thu, Oct 13, 2022 at 06:43:50PM +0000, anlauf at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483
> 
> --- Comment #7 from anlauf at gcc dot gnu.org ---
> (In reply to kargl from comment #6)
> > Harald, I looked at your patch and agree that simplification should be done.
> > I don't know why I did not do it when I wrote walk_array_constructor().
> 
> Because you had only nice Fortran code in mind and for testing?
> 

Yep!  This probably is the root of 90% of Gerhard's bug reports.

If regtesting complete ok, and Mikael doesn't find any additional
problems.  Please commit.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #9 from anlauf at gcc dot gnu.org ---
(In reply to Steve Kargl from comment #8)
> If regtesting complete ok,

This is the case.

> and Mikael doesn't find any additional problems.  Please commit.

The only thing I was fearing^Wexpecting is that he finds a better place
for the fix.

When varying various related testcases, one encounters from time to time
the error "Array operands are incommensurate" (ARITH_INCOMMENSURATE).
This seems to go back all the way to the beginnings of gfortran (r0-58166),
and there is no helpful commit message.  However, this error message can
lead to confusion, and there are related PRs.

Do you know the history of this?

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #10 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Thu, Oct 13, 2022 at 07:09:28PM +0000, anlauf at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483
> 
> --- Comment #9 from anlauf at gcc dot gnu.org ---
> (In reply to Steve Kargl from comment #8)
> > If regtesting complete ok,
> 
> This is the case.
> 
> > and Mikael doesn't find any additional problems.  Please commit.
> 
> The only thing I was fearing^Wexpecting is that he finds a better place
> for the fix.
> 
> When varying various related testcases, one encounters from time to time
> the error "Array operands are incommensurate" (ARITH_INCOMMENSURATE).
> This seems to go back all the way to the beginnings of gfortran (r0-58166),
> and there is no helpful commit message.  However, this error message can
> lead to confusion, and there are related PRs.
> 
> Do you know the history of this?
> 

Unfortunately, no.  I believe g95 came into GCC as gfortran in the
tree-ssa branch (under cvs).  At one time, one could check on-line
the branches under subversion.  I cannot find a tree-ssa branch under
git with the on-line repository.  I suspect this dates back to g95, 
and falls under the development where one assumes only correctly
written standard conforming Fortran is fed into gfortran.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikael at gcc dot gnu.org

--- Comment #11 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Steve Kargl from comment #8)
> If regtesting complete ok, and Mikael doesn't find any additional
> problems.  Please commit.

Unfortunately, I think there is one problem.
Generally speaking, I don't like doing too much at parsing stage, and
especially simplifying.  It's too early in my opinion.

Here is an example, where the array simplifies using the host-associated
parameter value instead of calling the contained function with the same name
hiding it.  It is admittedly somewhat artificial.

module m
  implicit none
  integer, parameter :: a(*) = [ 7, 11 ]
contains
  subroutine bug
    real :: b(1)
    b = [ real :: (a(1)) ]
    print *, b
    if (any(b /= [ 14. ])) stop 1
  contains
    function a(c)
      integer :: a, c
      a = c + 13
    end function a
  end subroutine bug
end module m

program p
  use m
  call bug
end program p

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #12 from anlauf at gcc dot gnu.org ---
(In reply to Mikael Morin from comment #11)
> Here is an example, where the array simplifies using the host-associated
> parameter value instead of calling the contained function with the same name
> hiding it.  It is admittedly somewhat artificial.

But it is a valid (counter-)example ... :-(

Symptoms: invalid declarations, such as

    real :: b(1) = [ real :: (a(1)) ]

or

    real, parameter :: b(1) = [ real :: (a(1)) ]

are not diagnosed, also not by the Intel compiler, but by NAG:

Error: pr93483-mm.f90, line 6: Reference to non-intrinsic function A in
constant expression

So this looks like a can of worms...

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #13 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Thu, Oct 13, 2022 at 07:35:30PM +0000, mikael at gcc dot gnu.org wrote:
> 
> --- Comment #11 from Mikael Morin <mikael at gcc dot gnu.org> ---
> (In reply to Steve Kargl from comment #8)
> > If regtesting complete ok, and Mikael doesn't find any additional
> > problems.  Please commit.
> 
> Unfortunately, I think there is one problem.
> Generally speaking, I don't like doing too much at parsing stage, and
> especially simplifying.  It's too early in my opinion.
> 
> Here is an example, where the array simplifies using the host-associated
> parameter value instead of calling the contained function with the same name
> hiding it.  It is admittedly somewhat artificial.
> 

If might be artificial, but "wrong code" is not a pleasant
side effect.

There is array.cc:gfc_resolve_array_constructor().  I haven't
looked to see when, or even if, this is called when compiling
the examples that Gerhard, Harald, and you have devised.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #14 from anlauf at gcc dot gnu.org ---
Is it conceivable that a somewhat weaker form of simplification, which
addresses the parentheses as well as the basic unary and binary operators
could still be used for the time being?

There is simplify_intrinsic_op; it just needs to be made non-static.
And seems to work here, also for Mikael's example.

So on top of the posted patch,

diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index 9bec299f160..4e937a4990f 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -1207,7 +1207,7 @@ walk_array_constructor (gfc_typespec *ts,
gfc_constructor_base head)
       e = c->expr;

       if (e->expr_type == EXPR_OP)
-       gfc_simplify_expr (e, 0);
+       simplify_intrinsic_op (e, 0);

       if (e->expr_type == EXPR_ARRAY && e->ts.type == BT_UNKNOWN
          && !e->ref && e->value.constructor)
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 10bb098d136..37dceacbb54 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3627,6 +3627,7 @@ gfc_expr *gfc_build_conversion (gfc_expr *);
 void gfc_free_ref_list (gfc_ref *);
 void gfc_type_convert_binary (gfc_expr *, int);
 bool gfc_is_constant_expr (gfc_expr *);
+bool simplify_intrinsic_op (gfc_expr *, int);
 bool gfc_simplify_expr (gfc_expr *, int);
 bool gfc_try_simplify_expr (gfc_expr *, int);
 int gfc_has_vector_index (gfc_expr *);

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #15 from anlauf at gcc dot gnu.org ---
(In reply to anlauf from comment #14)
> Is it conceivable that a somewhat weaker form of simplification, which
> addresses the parentheses as well as the basic unary and binary operators
> could still be used for the time being?
> 
> There is simplify_intrinsic_op; it just needs to be made non-static.
> And seems to work here, also for Mikael's example.
> 
> So on top of the posted patch,

Argh, this doesn't work.  My fault.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #16 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Thu, Oct 13, 2022 at 08:56:55PM +0000, anlauf at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483
> 
> --- Comment #15 from anlauf at gcc dot gnu.org ---
> (In reply to anlauf from comment #14)
> > Is it conceivable that a somewhat weaker form of simplification, which
> > addresses the parentheses as well as the basic unary and binary operators
> > could still be used for the time being?
> > 
> > There is simplify_intrinsic_op; it just needs to be made non-static.
> > And seems to work here, also for Mikael's example.
> > 
> > So on top of the posted patch,
> 
> Argh, this doesn't work.  My fault.
> 

gfc_simplify_expr will call simplify_intrinsic_op.  Thisi, I think,
leads to arith.cc:gfc_uplus and arith.cc:gfc_parentheses.  These
simply copy the expression to a new expression without trying to reduce
the expression, and then returns.  For unary plus, instead of copying
the expression, gfortran could copy the operand.  For parentheses,
one would need to walk the expression to see if it can be reduced.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #17 from Mikael Morin <mikael at gcc dot gnu.org> ---
There is the possibility to bail out at the very point where things are about
to go wrong, and hope that at resolution time simplification will happen.
Like this for the first part of the test from the patch:

diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index 9e079e42995..5e96bb9658e 100644
--- a/gcc/fortran/arith.cc
+++ b/gcc/fortran/arith.cc
@@ -1267,6 +1267,9 @@ reduce_unary (arith (*eval) (gfc_expr *, gfc_expr **),
gfc_expr *op,
   if (op->expr_type == EXPR_CONSTANT)
     return eval (op, result);

+  if (op->expr_type != EXPR_ARRAY)
+    return ARITH_INVALID_TYPE;
+
   rc = ARITH_OK;
   head = gfc_constructor_copy (op->value.constructor);
   for (c = gfc_constructor_first (head); c; c = gfc_constructor_next (c))


And something similar for the rest of the test (the binary operators).

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #18 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #17)
> And something similar for the rest of the test (the binary operators).

Like this:

diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index 5e96bb9658e..3fdda045227 100644
--- a/gcc/fortran/arith.cc
+++ b/gcc/fortran/arith.cc
@@ -1318,7 +1318,7 @@ reduce_binary_ac (arith (*eval) (gfc_expr *, gfc_expr *,
gfc_expr **),

       if (c->expr->expr_type == EXPR_CONSTANT)
         rc = eval (c->expr, op2, &r);
-      else if (c->expr->expr_type == EXPR_OP && c->expr->ts.type ==
BT_UNKNOWN)
+      else if (c->expr->expr_type != EXPR_ARRAY)
        rc = ARITH_INVALID_TYPE;
       else
        rc = reduce_binary_ac (eval, c->expr, op2, &r);
@@ -1372,7 +1372,7 @@ reduce_binary_ca (arith (*eval) (gfc_expr *, gfc_expr *,
gfc_expr **),

       if (c->expr->expr_type == EXPR_CONSTANT)
        rc = eval (op1, c->expr, &r);
-      else if (c->expr->expr_type == EXPR_OP && c->expr->ts.type ==
BT_UNKNOWN)
+      else if (c->expr->expr_type != EXPR_ARRAY)
        rc = ARITH_INVALID_TYPE;
       else
        rc = reduce_binary_ca (eval, op1, c->expr, &r);
@@ -1433,8 +1433,8 @@ reduce_binary_aa (arith (*eval) (gfc_expr *, gfc_expr *,
gfc_expr **),
        c && d;
        c = gfc_constructor_next (c), d = gfc_constructor_next (d))
     {
-      if ((c->expr->expr_type == EXPR_OP && c->expr->ts.type == BT_UNKNOWN)
-         || (d->expr->expr_type == EXPR_OP && d->expr->ts.type == BT_UNKNOWN))
+      if ((c->expr->expr_type != EXPR_ARRAY)
+         || (d->expr->expr_type != EXPR_ARRAY))
        rc = ARITH_INVALID_TYPE;
       else
        rc = reduce_binary (eval, c->expr, d->expr, &r);

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #19 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #18)
> (In reply to Mikael Morin from comment #17)
> > And something similar for the rest of the test (the binary operators).
> 
> Like this:
> 
It doesn't work unfortunately:

array_constructor_56.f90:18:28:

   18 |   real, parameter :: r7(*) =  [real :: 1, 2] * [real :: 1, (2)]
      |                            1
Error: Bad shape of initializer at (1)

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #20 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #19)
> (In reply to Mikael Morin from comment #18)
> > (In reply to Mikael Morin from comment #17)
> > > And something similar for the rest of the test (the binary operators).
> > 
> > Like this:
> > 
> It doesn't work unfortunately:
> 
This one works:

diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index 5e96bb9658e..c51ae105387 100644
--- a/gcc/fortran/arith.cc
+++ b/gcc/fortran/arith.cc
@@ -1318,7 +1318,7 @@ reduce_binary_ac (arith (*eval) (gfc_expr *, gfc_expr *,
gfc_expr **),

       if (c->expr->expr_type == EXPR_CONSTANT)
         rc = eval (c->expr, op2, &r);
-      else if (c->expr->expr_type == EXPR_OP && c->expr->ts.type ==
BT_UNKNOWN)
+      else if (c->expr->expr_type != EXPR_ARRAY)
        rc = ARITH_INVALID_TYPE;
       else
        rc = reduce_binary_ac (eval, c->expr, op2, &r);
@@ -1372,7 +1372,7 @@ reduce_binary_ca (arith (*eval) (gfc_expr *, gfc_expr *,
gfc_expr **),

       if (c->expr->expr_type == EXPR_CONSTANT)
        rc = eval (op1, c->expr, &r);
-      else if (c->expr->expr_type == EXPR_OP && c->expr->ts.type ==
BT_UNKNOWN)
+      else if (c->expr->expr_type != EXPR_ARRAY)
        rc = ARITH_INVALID_TYPE;
       else
        rc = reduce_binary_ca (eval, op1, c->expr, &r);
@@ -1433,8 +1433,8 @@ reduce_binary_aa (arith (*eval) (gfc_expr *, gfc_expr *,
gfc_expr **),
        c && d;
        c = gfc_constructor_next (c), d = gfc_constructor_next (d))
     {
-      if ((c->expr->expr_type == EXPR_OP && c->expr->ts.type == BT_UNKNOWN)
-         || (d->expr->expr_type == EXPR_OP && d->expr->ts.type == BT_UNKNOWN))
+      if ((!(c->expr->expr_type == EXPR_CONSTANT || c->expr->expr_type ==
EXPR_ARRAY))
+         || (!(d->expr->expr_type == EXPR_CONSTANT || d->expr->expr_type ==
EXPR_ARRAY)))
        rc = ARITH_INVALID_TYPE;
       else
        rc = reduce_binary (eval, c->expr, d->expr, &r);

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #21 from anlauf at gcc dot gnu.org ---
(In reply to Mikael Morin from comment #17)
> Like this for the first part of the test from the patch:
> 
> diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
> index 9e079e42995..5e96bb9658e 100644
> --- a/gcc/fortran/arith.cc
> +++ b/gcc/fortran/arith.cc
> @@ -1267,6 +1267,9 @@ reduce_unary (arith (*eval) (gfc_expr *, gfc_expr **),
> gfc_expr *op,
>    if (op->expr_type == EXPR_CONSTANT)
>      return eval (op, result);
>  
> +  if (op->expr_type != EXPR_ARRAY)
> +    return ARITH_INVALID_TYPE;
> +
>    rc = ARITH_OK;
>    head = gfc_constructor_copy (op->value.constructor);
>    for (c = gfc_constructor_first (head); c; c = gfc_constructor_next (c))
> 
> 

Yeah, I was getting just rather close to this one...

This is probably the right way to go.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #22 from anlauf at gcc dot gnu.org ---
(In reply to Mikael Morin from comment #11)
> Here is an example, where the array simplifies using the host-associated
> parameter value instead of calling the contained function with the same name
> hiding it.  It is admittedly somewhat artificial.

As this is something that is not yet covered by the testsuite, we should
really convert it to a testcase and add it. (array_constructor_57.f90?)

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #23 from anlauf at gcc dot gnu.org ---
Created attachment 53706
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53706&action=edit
Updated patch

Here's a patch that incorporates comment#17 and comment#20 and adds a testcase
for comment #11.  Regtests fine.

@Mikael: is it what you suggested?

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #24 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to anlauf from comment #21)
> 
> Yeah, I was getting just rather close to this one...
> 
Sorry, I didn't want to take it out of your hands.
It seemed that no real solution was emerging.

(In reply to anlauf from comment #23)
> Here's a patch that incorporates comment#17 and comment#20 and adds a
> testcase
> for comment #11.  Regtests fine.
> 
> @Mikael: is it what you suggested?

Yes.
It gives a general direction, but there are a couple of things that I'm not
satisfied with.

First, the ARITH_INVALID_TYPE should be renamed as it has now a broader usage
(ARITH_OP_NOT_LITERAL_VALUE is a bit long, ARITH_OP_NOT_CONSTANT is a bit
misleading, ARITH_OP_NOT_SIMPLIFIED not great either, any other idea?).

Second, I'm wondering whether the check in reduce_binary_aa shouldn't be moved
to reduce binary where it would be more clear.  But then it would be less
consistent with reduce_binary_ac and reduce_binary_ca.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #25 from anlauf at gcc dot gnu.org ---
(In reply to Mikael Morin from comment #24)
> First, the ARITH_INVALID_TYPE should be renamed as it has now a broader
> usage (ARITH_OP_NOT_LITERAL_VALUE is a bit long, ARITH_OP_NOT_CONSTANT is a
> bit misleading, ARITH_OP_NOT_SIMPLIFIED not great either, any other idea?).

I think we should keep the enum ARITH_INVALID_TYPE for those cases where it
is appropriate, and have a different enum for cases where reduce_* does not
succeed.  I was contemplating either ARITH_NOT_REDUCED or ARITH_CANNOT_REDUCE,
and opted for the latter.

> Second, I'm wondering whether the check in reduce_binary_aa shouldn't be
> moved to reduce binary where it would be more clear.

I agree that it is preferable to have checks already in reduce_binary, see
updated patch.  After this one could remove the check from reduce_binary_aa,
as it would be redundant.

> But then it would be
> less consistent with reduce_binary_ac and reduce_binary_ca.

I wouldn't say "less consistent", it just looks less symmetric,
but this is only so as long as you ignore the code in reduce_binary.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #53706|0                           |1
        is obsolete|                            |

--- Comment #26 from anlauf at gcc dot gnu.org ---
Created attachment 53708
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53708&action=edit
Updated patch.

Passes basic testing.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #27 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to anlauf from comment #25)
> (In reply to Mikael Morin from comment #24)
> > First, the ARITH_INVALID_TYPE should be renamed as it has now a broader
> > usage (ARITH_OP_NOT_LITERAL_VALUE is a bit long, ARITH_OP_NOT_CONSTANT is a
> > bit misleading, ARITH_OP_NOT_SIMPLIFIED not great either, any other idea?).
> 
> I think we should keep the enum ARITH_INVALID_TYPE for those cases where it
> is appropriate,

Are there such cases remaining?  It seems that that value can't be returned any
more.

> I was contemplating either ARITH_NOT_REDUCED or
> ARITH_CANNOT_REDUCE,
> and opted for the latter.
> 
I have a slight preference for the former but let's go with the latter if you
prefer.  But please add a comment describing it in the definition.
Most enum values have an obvious meaning there, this one less so.


> > Second, I'm wondering whether the check in reduce_binary_aa shouldn't be
> > moved to reduce binary where it would be more clear.
> 
> I agree that it is preferable to have checks already in reduce_binary, see
> updated patch.  After this one could remove the check from reduce_binary_aa,
> as it would be redundant.
> 
And there is a redundant check in reduce_unary as well.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #28 from anlauf at gcc dot gnu.org ---
(In reply to Mikael Morin from comment #27)
> (In reply to anlauf from comment #25)
> > (In reply to Mikael Morin from comment #24)
> > > First, the ARITH_INVALID_TYPE should be renamed as it has now a broader
> > > usage (ARITH_OP_NOT_LITERAL_VALUE is a bit long, ARITH_OP_NOT_CONSTANT is a
> > > bit misleading, ARITH_OP_NOT_SIMPLIFIED not great either, any other idea?).
> > 
> > I think we should keep the enum ARITH_INVALID_TYPE for those cases where it
> > is appropriate,
> 
> Are there such cases remaining?  It seems that that value can't be returned
> any more.

Actually there is the fix for pr107217, and I found a couple of other similar
cases involving comparisons and logicals which are not yet handled.
I will submit a patch.

> > I was contemplating either ARITH_NOT_REDUCED or
> > ARITH_CANNOT_REDUCE,
> > and opted for the latter.
> > 
> I have a slight preference for the former but let's go with the latter if you
> prefer.  But please add a comment describing it in the definition.
> Most enum values have an obvious meaning there, this one less so.

Will do.

> > > Second, I'm wondering whether the check in reduce_binary_aa shouldn't be
> > > moved to reduce binary where it would be more clear.
> > 
> > I agree that it is preferable to have checks already in reduce_binary, see
> > updated patch.  After this one could remove the check from reduce_binary_aa,
> > as it would be redundant.
> > 
> And there is a redundant check in reduce_unary as well.

True.  I will submit an updated patch.

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

--- Comment #29 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>:

https://gcc.gnu.org/g:d45af5c2eb1ba1e48449d8f3c5b4e3994a956f92

commit r13-3340-gd45af5c2eb1ba1e48449d8f3c5b4e3994a956f92
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Sat Oct 15 21:56:56 2022 +0200

    Fortran: handle bad array ctors with typespec [PR93483, PR107216, PR107219]

    gcc/fortran/ChangeLog:

            PR fortran/93483
            PR fortran/107216
            PR fortran/107219
            * arith.cc (reduce_unary): Handled expressions are EXP_CONSTANT and
            EXPR_ARRAY.  Do not attempt to reduce otherwise.
            (reduce_binary_ac): Likewise.
            (reduce_binary_ca): Likewise.
            (reduce_binary_aa): Moved check for EXP_CONSTANT and EXPR_ARRAY
            from here ...
            (reduce_binary): ... to here.
            (eval_intrinsic): Catch failed reductions.
            * gfortran.h (GFC_INTRINSIC_OPS): New enum ARITH_NOT_REDUCED to
keep
            track of expressions that were not reduced by the arithmetic
evaluation
            code.

    gcc/testsuite/ChangeLog:

            PR fortran/93483
            PR fortran/107216
            PR fortran/107219
            * gfortran.dg/array_constructor_56.f90: New test.
            * gfortran.dg/array_constructor_57.f90: New test.

    Co-authored-by: Mikael Morin <mikael@gcc.gnu.org>

[Bug fortran/93483] ICE in gfc_constructor_copy, at fortran/constructor.c:103

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93483

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #30 from anlauf at gcc dot gnu.org ---
Fixed for gcc-13.  Closing.

Thanks for insisting to look into this!