[Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

Hi all,

the cause of this PR was that the array spec of a vtype's PPC was not
correctly resolved. The attached patch corrects this and does some
related cleanup in three places:

1) In resolve_fl_derived, I'm replacing a piece of code by a call to
gfc_resolve_array_spec, which should do exactly the same checks. Plus:
It should correctly resolve the array spec for PPCs.

2) In resolve_array_bound, I'm modifying an error message which
assumed that the expression of the array bound is always a variable
(which is clearly wrong and results in an ICE when the expression is
not a variable).

3) In gfc_is_constant_expr, I'm adding a special case for the RAND()
and IRAND() intrinsic functions, which were detected to be constant
before. [I'm assuming that a "constant expression" is something that
can be reduced to a constant at compile time, which the random
functions are clearly not.]

The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?

Cheers,
Janus



2010-07-18  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/44962
	* array.c (resolve_array_bound): Modify error message.
	* expr.c (gfc_is_constant_expr): Detect RAND() and IRAND() as
	non-constant.
	* resolve.c (resolve_fl_derived): Call gfc_resolve_array_spec.


2010-07-18  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/44962
	* gfortran.dg/typebound_proc_17.f03: New.

[-- Attachment #2: pr44962_v2.diff --]
[-- Type: application/octet-stream, Size: 2722 bytes --]

Index: gcc/fortran/array.c
===================================================================
--- gcc/fortran/array.c	(revision 162282)
+++ gcc/fortran/array.c	(working copy)
@@ -302,8 +302,7 @@ resolve_array_bound (gfc_expr *e, int check_consta
 
   if (check_constant && gfc_is_constant_expr (e) == 0)
     {
-      gfc_error ("Variable '%s' at %L in this context must be constant",
-		 e->symtree->n.sym->name, &e->where);
+      gfc_error ("Array bound at %L must be constant", &e->where);
       return FAILURE;
     }
 
Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 162282)
+++ gcc/fortran/expr.c	(working copy)
@@ -925,10 +925,18 @@ gfc_is_constant_expr (gfc_expr *e)
       /* Call to intrinsic with at least one argument.  */
       if (e->value.function.isym && e->value.function.actual)
 	{
-	  for (arg = e->value.function.actual; arg; arg = arg->next)
-	    if (!gfc_is_constant_expr (arg->expr))
+	  switch (e->value.function.isym->id)
+	    {
+	    case GFC_ISYM_IRAND:
+	    case GFC_ISYM_RAND:
+	      /* RNG functions are by definition not constant.  */
 	      return 0;
-
+	    default:
+	      /* Check for constant arguments.  */
+	      for (arg = e->value.function.actual; arg; arg = arg->next)
+		if (!gfc_is_constant_expr (arg->expr))
+		  return 0; 
+	    }
 	  return 1;
 	}
       else
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 162282)
+++ gcc/fortran/resolve.c	(working copy)
@@ -10733,7 +10733,6 @@ resolve_fl_derived (gfc_symbol *sym)
 {
   gfc_symbol* super_type;
   gfc_component *c;
-  int i;
 
   super_type = gfc_get_derived_super_type (sym);
   
@@ -11089,25 +11088,10 @@ resolve_fl_derived (gfc_symbol *sym)
 	    && sym != c->ts.u.derived)
 	add_dt_to_dt_list (c->ts.u.derived);
 
-      if (c->attr.pointer || c->attr.proc_pointer || c->attr.allocatable
-	  || c->as == NULL)
-	continue;
-
-      for (i = 0; i < c->as->rank; i++)
-	{
-	  if (c->as->lower[i] == NULL
-	      || (resolve_index_expr (c->as->lower[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->lower[i])
-	      || c->as->upper[i] == NULL
-	      || (resolve_index_expr (c->as->upper[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->upper[i]))
-	    {
-	      gfc_error ("Component '%s' of '%s' at %L must have "
-			 "constant array bounds",
-			 c->name, sym->name, &c->loc);
-	      return FAILURE;
-	    }
-	}
+      if (gfc_resolve_array_spec (c->as, !(c->attr.pointer
+					   || c->attr.proc_pointer
+					   || c->attr.allocatable)) == FAILURE)
+	return FAILURE;
     }
 
   /* Resolve the type-bound procedures.  */

[-- Attachment #3: typebound_proc_17.f03 --]
[-- Type: application/octet-stream, Size: 596 bytes --]

! { dg-do compile }
!
! PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)
!
! Contributed by Satish.BD <bdsatish@gmail.com>


module array

type :: t_array
  real, dimension(10) :: coeff
contains
  procedure :: get_coeff
end type t_array

contains

function get_coeff(self) result(coeff)
  class(t_array), intent(in) :: self
  real, dimension(size(self%coeff)) :: coeff !! The SIZE here carashes !!
end function get_coeff

end module array


type :: t2
  real, dimension(irand()+5) :: co2  ! { dg-error "must be constant" }
end type

end

! { dg-final { cleanup-modules "array" } }

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Steve Kargl]

On Sun, Jul 18, 2010 at 06:36:39PM +0200, Janus Weil wrote:
> 
> 3) In gfc_is_constant_expr, I'm adding a special case for the RAND()
> and IRAND() intrinsic functions, which were detected to be constant
> before. [I'm assuming that a "constant expression" is something that
> can be reduced to a constant at compile time, which the random
> functions are clearly not.]

I think that this part is the wrong way to address the issue.
In looking at intrinsics.c, I see that neither rand nor irand
have simplification procedures.  I would presume that a default
simplification routine is called, but I haven't yet traced
through a toy code to be certain.

-- 
Steve

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Janus Weil]

Hi Steve,

>> 3) In gfc_is_constant_expr, I'm adding a special case for the RAND()
>> and IRAND() intrinsic functions, which were detected to be constant
>> before. [I'm assuming that a "constant expression" is something that
>> can be reduced to a constant at compile time, which the random
>> functions are clearly not.]
>
> I think that this part is the wrong way to address the issue.

what do you think would be a better way?


> In looking at intrinsics.c, I see that neither rand nor irand
> have simplification procedures.

Of course not. How could they? They will not be simplifyable at
compile time in any way ...


Cheers,
Janus

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Steve Kargl]

On Sun, Jul 18, 2010 at 07:55:13PM +0200, Janus Weil wrote:
> Hi Steve,
> 
> >> 3) In gfc_is_constant_expr, I'm adding a special case for the RAND()
> >> and IRAND() intrinsic functions, which were detected to be constant
> >> before. [I'm assuming that a "constant expression" is something that
> >> can be reduced to a constant at compile time, which the random
> >> functions are clearly not.]
> >
> > I think that this part is the wrong way to address the issue.
> 
> what do you think would be a better way?

I don't know, yet.  But, special casing these two intrinsics
in gfc_is_constant_expr seems wrong.  With hundreds of intrinsics
procedures, are these the only 2 that need this special handling?
If yes, then why?  If no, then we need to add special casing for
all intrinsics.

> > In looking at intrinsics.c, I see that neither rand nor irand
> > have simplification procedures.
> 
> Of course not. How could they? They will not be simplifyable at
> compile time in any way ...

Sorry, misread your comment.  I was thinking about something
like 'i = irand(j)' verseus 'i = irand(23)' where the argument
is optional.

With the toy programs I've written this morning involving rand(),
f951 never enter gfc_is_constant_expr().  Do you have a small
example that exhibits this problem?

-- 
Steve

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Janus Weil]

2010/7/18 Steve Kargl <sgk@troutmask.apl.washington.edu>:
> On Sun, Jul 18, 2010 at 07:55:13PM +0200, Janus Weil wrote:
>> Hi Steve,
>>
>> >> 3) In gfc_is_constant_expr, I'm adding a special case for the RAND()
>> >> and IRAND() intrinsic functions, which were detected to be constant
>> >> before. [I'm assuming that a "constant expression" is something that
>> >> can be reduced to a constant at compile time, which the random
>> >> functions are clearly not.]
>> >
>> > I think that this part is the wrong way to address the issue.
>>
>> what do you think would be a better way?
>
> I don't know, yet.  But, special casing these two intrinsics
> in gfc_is_constant_expr seems wrong.  With hundreds of intrinsics
> procedures, are these the only 2 that need this special handling?
> If yes, then why?

Actually I've been asking myself the same question. I guess the
random-number functions are special in the sense that they are not
deterministic. I.e. all other functions will always give you the same
output when fed with the same input, meaning they will be constants if
their arguments are constants. This is not the case for RAND and
IRAND. [They are the only RNG intrinsic functions I found. Hope I
missed none.]


>> > In looking at intrinsics.c, I see that neither rand nor irand
>> > have simplification procedures.
>>
>> Of course not. How could they? They will not be simplifyable at
>> compile time in any way ...
>
> Sorry, misread your comment.  I was thinking about something
> like 'i = irand(j)' verseus 'i = irand(23)' where the argument
> is optional.
>
> With the toy programs I've written this morning involving rand(),
> f951 never enter gfc_is_constant_expr().  Do you have a small
> example that exhibits this problem?

See the test case that I attached along with the patch. It has:

type :: t2
  real, dimension(irand()+5) :: co2  ! { dg-error "must be constant" }
end type


Cheers,
Janus

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Tobias Burnus]

Janus Weil wrote:

> 2010-07-18  Janus Weil  <janus@gcc.gnu.org>
>
> 	PR fortran/44962
> 	* array.c (resolve_array_bound): Modify error message.
> 	* resolve.c (resolve_fl_derived): Call gfc_resolve_array_spec.
>   
These two changes are OK.

> 	* expr.c (gfc_is_constant_expr): Detect RAND() and IRAND() as
> 	non-constant.
>
> 3) In gfc_is_constant_expr, I'm adding a special case for the RAND()
> and IRAND() intrinsic functions, which were detected to be constant
> before. [I'm assuming that a "constant expression" is something that
> can be reduced to a constant at compile time, which the random
> functions are clearly not.]
>   

Frankly, I do not quite understand the check. I know

a) "initialization expressions", which in Fortran 90 and 2008 are called
"constant expressions", i.e. something which can be reduced at compile
time to a number or string (or an array (constructor) of those).
b) "Specification expressions", which do not need to be constant, but
which need to fulfil some criteria (pureness etc.)

Howver, I have not quite understood how this maps to expr.c's
init/restricted/specification/constant expressions.

Especially, I would expect that gfc_is_constant_expr match an expression
yielding a constant. However, this does not seem to be the case as
specification expressions are allowed. The check for intrinsic functions
I also do not understand. The comment states that a simplified
expression is required. However, if the intrinsic can be simplified,
only has a simple number - and no EXPR_FUNCTION any more. And if not,
well, then the check of the arguments does also not help.

Tobias

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 1714 bytes --]

Hi Tobias,

> Frankly, I do not quite understand the check. I know
>
> a) "initialization expressions", which in Fortran 90 and 2008 are called
> "constant expressions", i.e. something which can be reduced at compile
> time to a number or string (or an array (constructor) of those).
> b) "Specification expressions", which do not need to be constant, but
> which need to fulfil some criteria (pureness etc.)

Well, in general array bounds are only required to be specification
expressions (F08:R516-518). However, there is an additional
constraint. In F03 it was:

C542 (R511) An explicit-shape array whose bounds are not
initialization expressions shall be a dummy argument, a function
result, or an automatic array of a procedure.

In F08 it is:

C531 (R516) An explicit-shape-spec whose bounds are not constant
expressions shall appear only in a subpro-
gram, derived type de\ffinition, BLOCK construct, or interface body.

I think this restriction is the reason that we sometimes check for
constant expressions (cf. the argument 'check_constant' of
'gfc_resolve_array_spec'). If I am interpreting the above restrictions
correctly, then components had to have constant array bounds in F03,
while in F08 they don't have to be constant. Is that right?


> The check for intrinsic functions
> I also do not understand. The comment states that a simplified
> expression is required. However, if the intrinsic can be simplified,
> only has a simple number - and no EXPR_FUNCTION any more. And if not,
> well, then the check of the arguments does also not help.

Good point. So, what do you think about this new version of the patch?
I will regtest it now ...

Cheers,
Janus

[-- Attachment #2: pr44962_v3.diff --]
[-- Type: application/octet-stream, Size: 2574 bytes --]

Index: gcc/fortran/array.c
===================================================================
--- gcc/fortran/array.c	(revision 162282)
+++ gcc/fortran/array.c	(working copy)
@@ -302,8 +302,7 @@ resolve_array_bound (gfc_expr *e, int check_consta
 
   if (check_constant && gfc_is_constant_expr (e) == 0)
     {
-      gfc_error ("Variable '%s' at %L in this context must be constant",
-		 e->symtree->n.sym->name, &e->where);
+      gfc_error ("Array bound at %L must be constant", &e->where);
       return FAILURE;
     }
 
Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 162282)
+++ gcc/fortran/expr.c	(working copy)
@@ -900,7 +900,6 @@ int
 gfc_is_constant_expr (gfc_expr *e)
 {
   gfc_constructor *c;
-  gfc_actual_arglist *arg;
 
   if (e == NULL)
     return 1;
@@ -921,16 +920,6 @@ gfc_is_constant_expr (gfc_expr *e)
       /* Specification functions are constant.  */
       if (check_specification_function (e) == MATCH_YES)
 	return 1;
-
-      /* Call to intrinsic with at least one argument.  */
-      if (e->value.function.isym && e->value.function.actual)
-	{
-	  for (arg = e->value.function.actual; arg; arg = arg->next)
-	    if (!gfc_is_constant_expr (arg->expr))
-	      return 0;
-
-	  return 1;
-	}
       else
 	return 0;
 
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 162282)
+++ gcc/fortran/resolve.c	(working copy)
@@ -10733,7 +10733,6 @@ resolve_fl_derived (gfc_symbol *sym)
 {
   gfc_symbol* super_type;
   gfc_component *c;
-  int i;
 
   super_type = gfc_get_derived_super_type (sym);
   
@@ -11089,25 +11088,8 @@ resolve_fl_derived (gfc_symbol *sym)
 	    && sym != c->ts.u.derived)
 	add_dt_to_dt_list (c->ts.u.derived);
 
-      if (c->attr.pointer || c->attr.proc_pointer || c->attr.allocatable
-	  || c->as == NULL)
-	continue;
-
-      for (i = 0; i < c->as->rank; i++)
-	{
-	  if (c->as->lower[i] == NULL
-	      || (resolve_index_expr (c->as->lower[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->lower[i])
-	      || c->as->upper[i] == NULL
-	      || (resolve_index_expr (c->as->upper[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->upper[i]))
-	    {
-	      gfc_error ("Component '%s' of '%s' at %L must have "
-			 "constant array bounds",
-			 c->name, sym->name, &c->loc);
-	      return FAILURE;
-	    }
-	}
+      if (gfc_resolve_array_spec (c->as, false) == FAILURE)
+	return FAILURE;
     }
 
   /* Resolve the type-bound procedures.  */

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Janus Weil]

>> The check for intrinsic functions
>> I also do not understand. The comment states that a simplified
>> expression is required. However, if the intrinsic can be simplified,
>> only has a simple number - and no EXPR_FUNCTION any more. And if not,
>> well, then the check of the arguments does also not help.
>
> Good point. So, what do you think about this new version of the patch?
> I will regtest it now ...

Regtest was successful.

Cheers,
Janus

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Tobias Burnus]

Hi Janus,

Janus Weil wrote:
>> Frankly, I do not quite understand the check. [...]
>>     
> Well, in general array bounds are only required to be specification
> expressions (F08:R516-518). However, there is an additional
> constraint. In F03 it was:
>
> C542 (R511) An explicit-shape array whose bounds are not
> initialization expressions shall be a dummy argument, a function
> result, or an automatic array of a procedure.
>
> In F08 it is:
>
> C531 (R516) An explicit-shape-spec whose bounds are not constant expressions shall appear only in a subprogram, derived type de\ffinition, BLOCK construct, or interface body.
>   

Can you add a comment before the gfc_is_constant_expr to state what the
function is actually doing (e.g. checking for specification expressions
while taking additionally the given constraints into account.)

> If I am interpreting the above restrictions correctly, then components
> had to have constant array bounds in F03, while in F08 they don't have
> to be constant. Is that right?

Seems so, but I somehow do not understand how this is supposed to work;
probably somehow like:

subroutine foo(n)
  type t
    integer :: a(n)
  end type t
...
end


> Good point. So, what do you think about this new version of the patch?
> I will regtest it now ...
>   

Somehow also the references look wrong. If I look at

check_specification_function

it claims that this is: "F95, 7.1.6.2; F2003, 7.1.7", but F2003's 7.1.7
is about initialization expressions; I assume it should be 7.1.6
(specification expressions).
(Please correct as well.)

As you check for specification expressions, you cannot simply remove the
check. The reason is that check_specification_function excludes
intrinsic functions (cf. "specification function" in the standard).
Thus, you still need to check for:

(7) A specification inquiry where each designator or function argument
is [...]
(8) A reference to any other standard intrinsic function where each
argument is a restricted expression,

I do not think that one needs to exclude the random functions (who says
that the array size needs to be deterministic). But I still do not
understand why there is currently a check for having minimally a single
argument.

Regarding your example: It seems to be valid in F2008 (assuming that
"irand()" is regarded as specification expression) but it seems to be
invalid in F2003 due to C542...

I think allowing non-constant local types will be fun for extensible
types and dynamic types, which - using BLOCK and internal functions one
can easily use.

Tobias

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 2916 bytes --]

>>> Frankly, I do not quite understand the check. [...]
>>>
>> Well, in general array bounds are only required to be specification
>> expressions (F08:R516-518). However, there is an additional
>> constraint. In F03 it was:
>>
>> C542 (R511) An explicit-shape array whose bounds are not
>> initialization expressions shall be a dummy argument, a function
>> result, or an automatic array of a procedure.
>>
>> In F08 it is:
>>
>> C531 (R516) An explicit-shape-spec whose bounds are not constant expressions shall appear only in a subprogram, derived type de finition, BLOCK construct, or interface body.
>>
>
> Can you add a comment before the gfc_is_constant_expr to state what the
> function is actually doing (e.g. checking for specification expressions
> while taking additionally the given constraints into account.)

Well, it's actually not checking for specification expressions (which
is what gfc_specification_expr does), but it checks for constant
expressions, which are defined in chapter 7.1.12 of F08.


>> If I am interpreting the above restrictions correctly, then components
>> had to have constant array bounds in F03, while in F08 they don't have
>> to be constant. Is that right?
>
> Seems so, but I somehow do not understand how this is supposed to work;
> probably somehow like:
>
> subroutine foo(n)
>  type t
>    integer :: a(n)
>  end type t
> ...
> end

Probably.


> As you check for specification expressions, you cannot simply remove the
> check. The reason is that check_specification_function excludes
> intrinsic functions (cf. "specification function" in the standard).

No. Firstly, in "gfc_is_constant_expr" we're not checking for
specification expressions. Second, constant expressions can *only*
contain intrinsic functions.

I think a "specification function" is a special case of a
"specification expression". AFAICS, specification expressions in
general can of course include intrinsic functions.

If one checks for constant expressions and demands that they have been
simplified, I think one can really reject all EXPR_FUNCTIONS: A
function in a constant expr must be intrinsic, and those should be
simplified away to a constant already. And no, specification functions
are not constant. Quite the opposite: They cannot be!

In fact, 'check_specification_function' can be removed, since it is
equivalent to 'external_spec_function', and is only used in this one
place where it is completely inappropriate.


> I do not think that one needs to exclude the random functions (who says
> that the array size needs to be deterministic).

Well, I think the intention of "constant expressions" is to be
something which can be evaluated to a constant at *compile* time. This
is not the case for non-deterministic random functions.

I'm now regtesting the attached patch. Any further comments?

Cheers,
Janus

[-- Attachment #2: pr44962_v4.diff --]
[-- Type: application/octet-stream, Size: 3615 bytes --]

Index: gcc/fortran/array.c
===================================================================
--- gcc/fortran/array.c	(revision 162282)
+++ gcc/fortran/array.c	(working copy)
@@ -302,8 +302,7 @@ resolve_array_bound (gfc_expr *e, int check_consta
 
   if (check_constant && gfc_is_constant_expr (e) == 0)
     {
-      gfc_error ("Variable '%s' at %L in this context must be constant",
-		 e->symtree->n.sym->name, &e->where);
+      gfc_error ("Array bound at %L must be constant", &e->where);
       return FAILURE;
     }
 
Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 162282)
+++ gcc/fortran/expr.c	(working copy)
@@ -868,39 +868,14 @@ done:
 }
 
 
-static match
-check_specification_function (gfc_expr *e)
-{
-  gfc_symbol *sym;
-
-  if (!e->symtree)
-    return MATCH_NO;
-
-  sym = e->symtree->n.sym;
-
-  /* F95, 7.1.6.2; F2003, 7.1.7  */
-  if (sym
-      && sym->attr.function
-      && sym->attr.pure
-      && !sym->attr.intrinsic
-      && !sym->attr.recursive
-      && sym->attr.proc != PROC_INTERNAL
-      && sym->attr.proc != PROC_ST_FUNCTION
-      && sym->attr.proc != PROC_UNKNOWN
-      && sym->formal == NULL)
-    return MATCH_YES;
-
-  return MATCH_NO;
-}
-
 /* Function to determine if an expression is constant or not.  This
-   function expects that the expression has already been simplified.  */
+   function expects that the expression has already been simplified.
+   Cf. F08, chapter 7.1.12.  */
 
 int
 gfc_is_constant_expr (gfc_expr *e)
 {
   gfc_constructor *c;
-  gfc_actual_arglist *arg;
 
   if (e == NULL)
     return 1;
@@ -913,27 +888,11 @@ gfc_is_constant_expr (gfc_expr *e)
 		  || gfc_is_constant_expr (e->value.op.op2)));
 
     case EXPR_VARIABLE:
-      return 0;
-
     case EXPR_FUNCTION:
     case EXPR_PPC:
     case EXPR_COMPCALL:
-      /* Specification functions are constant.  */
-      if (check_specification_function (e) == MATCH_YES)
-	return 1;
+      return 0;
 
-      /* Call to intrinsic with at least one argument.  */
-      if (e->value.function.isym && e->value.function.actual)
-	{
-	  for (arg = e->value.function.actual; arg; arg = arg->next)
-	    if (!gfc_is_constant_expr (arg->expr))
-	      return 0;
-
-	  return 1;
-	}
-      else
-	return 0;
-
     case EXPR_CONSTANT:
     case EXPR_NULL:
       return 1;
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 162282)
+++ gcc/fortran/resolve.c	(working copy)
@@ -10733,7 +10733,6 @@ resolve_fl_derived (gfc_symbol *sym)
 {
   gfc_symbol* super_type;
   gfc_component *c;
-  int i;
 
   super_type = gfc_get_derived_super_type (sym);
   
@@ -11089,25 +11088,8 @@ resolve_fl_derived (gfc_symbol *sym)
 	    && sym != c->ts.u.derived)
 	add_dt_to_dt_list (c->ts.u.derived);
 
-      if (c->attr.pointer || c->attr.proc_pointer || c->attr.allocatable
-	  || c->as == NULL)
-	continue;
-
-      for (i = 0; i < c->as->rank; i++)
-	{
-	  if (c->as->lower[i] == NULL
-	      || (resolve_index_expr (c->as->lower[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->lower[i])
-	      || c->as->upper[i] == NULL
-	      || (resolve_index_expr (c->as->upper[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->upper[i]))
-	    {
-	      gfc_error ("Component '%s' of '%s' at %L must have "
-			 "constant array bounds",
-			 c->name, sym->name, &c->loc);
-	      return FAILURE;
-	    }
-	}
+      if (gfc_resolve_array_spec (c->as, false) == FAILURE)
+	return FAILURE;
     }
 
   /* Resolve the type-bound procedures.  */

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

> I'm now regtesting the attached patch. Any further comments?

Regtest was successful. Ok for trunk?

Cheers,
Janus



2010-07-23  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/44962
	* array.c (resolve_array_bound): Modify error message.
	* expr.c (check_specification_function): Removed.
	(gfc_is_constant_expr): Simplified constant expressions should not
	contain EXPR_FUNC's any more.
	* resolve.c (resolve_fl_derived): Call gfc_resolve_array_spec.


2010-07-23  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/44962
	* gfortran.dg/typebound_proc_17.f03: New.

[-- Attachment #2: pr44962_v4.diff --]
[-- Type: application/octet-stream, Size: 3615 bytes --]

Index: gcc/fortran/array.c
===================================================================
--- gcc/fortran/array.c	(revision 162448)
+++ gcc/fortran/array.c	(working copy)
@@ -302,8 +302,7 @@ resolve_array_bound (gfc_expr *e, int check_consta
 
   if (check_constant && gfc_is_constant_expr (e) == 0)
     {
-      gfc_error ("Variable '%s' at %L in this context must be constant",
-		 e->symtree->n.sym->name, &e->where);
+      gfc_error ("Array bound at %L must be constant", &e->where);
       return FAILURE;
     }
 
Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 162448)
+++ gcc/fortran/expr.c	(working copy)
@@ -868,39 +868,14 @@ done:
 }
 
 
-static match
-check_specification_function (gfc_expr *e)
-{
-  gfc_symbol *sym;
-
-  if (!e->symtree)
-    return MATCH_NO;
-
-  sym = e->symtree->n.sym;
-
-  /* F95, 7.1.6.2; F2003, 7.1.7  */
-  if (sym
-      && sym->attr.function
-      && sym->attr.pure
-      && !sym->attr.intrinsic
-      && !sym->attr.recursive
-      && sym->attr.proc != PROC_INTERNAL
-      && sym->attr.proc != PROC_ST_FUNCTION
-      && sym->attr.proc != PROC_UNKNOWN
-      && sym->formal == NULL)
-    return MATCH_YES;
-
-  return MATCH_NO;
-}
-
 /* Function to determine if an expression is constant or not.  This
-   function expects that the expression has already been simplified.  */
+   function expects that the expression has already been simplified.
+   Cf. F08, chapter 7.1.12.  */
 
 int
 gfc_is_constant_expr (gfc_expr *e)
 {
   gfc_constructor *c;
-  gfc_actual_arglist *arg;
 
   if (e == NULL)
     return 1;
@@ -913,27 +888,11 @@ gfc_is_constant_expr (gfc_expr *e)
 		  || gfc_is_constant_expr (e->value.op.op2)));
 
     case EXPR_VARIABLE:
-      return 0;
-
     case EXPR_FUNCTION:
     case EXPR_PPC:
     case EXPR_COMPCALL:
-      /* Specification functions are constant.  */
-      if (check_specification_function (e) == MATCH_YES)
-	return 1;
+      return 0;
 
-      /* Call to intrinsic with at least one argument.  */
-      if (e->value.function.isym && e->value.function.actual)
-	{
-	  for (arg = e->value.function.actual; arg; arg = arg->next)
-	    if (!gfc_is_constant_expr (arg->expr))
-	      return 0;
-
-	  return 1;
-	}
-      else
-	return 0;
-
     case EXPR_CONSTANT:
     case EXPR_NULL:
       return 1;
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 162448)
+++ gcc/fortran/resolve.c	(working copy)
@@ -10793,7 +10793,6 @@ resolve_fl_derived (gfc_symbol *sym)
 {
   gfc_symbol* super_type;
   gfc_component *c;
-  int i;
 
   super_type = gfc_get_derived_super_type (sym);
   
@@ -11149,25 +11148,8 @@ resolve_fl_derived (gfc_symbol *sym)
 	    && sym != c->ts.u.derived)
 	add_dt_to_dt_list (c->ts.u.derived);
 
-      if (c->attr.pointer || c->attr.proc_pointer || c->attr.allocatable
-	  || c->as == NULL)
-	continue;
-
-      for (i = 0; i < c->as->rank; i++)
-	{
-	  if (c->as->lower[i] == NULL
-	      || (resolve_index_expr (c->as->lower[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->lower[i])
-	      || c->as->upper[i] == NULL
-	      || (resolve_index_expr (c->as->upper[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->upper[i]))
-	    {
-	      gfc_error ("Component '%s' of '%s' at %L must have "
-			 "constant array bounds",
-			 c->name, sym->name, &c->loc);
-	      return FAILURE;
-	    }
-	}
+      if (gfc_resolve_array_spec (c->as, false) == FAILURE)
+	return FAILURE;
     }
 
   /* Resolve the type-bound procedures.  */

[-- Attachment #3: typebound_proc_17.f03 --]
[-- Type: application/octet-stream, Size: 640 bytes --]

! { dg-do compile }
!
! PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)
!
! Contributed by Satish.BD <bdsatish@gmail.com>


module array

type :: t_array
  real, dimension(10) :: coeff
contains
  procedure :: get_coeff
end type t_array

contains

function get_coeff(self) result(coeff)
  class(t_array), intent(in) :: self
  real, dimension(size(self%coeff)) :: coeff !! The SIZE here carashes !!
end function get_coeff

end module array


type :: t2
  real, dimension(iabs(-3)+2) :: com
end type

real, dimension(irand()+2) :: r2   ! { dg-error "must have constant shape" }

end

! { dg-final { cleanup-modules "array" } }

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Tobias Burnus]

Janus Weil wrote:
>> Can you add a comment before the gfc_is_constant_expr to state what the
>> function is actually doing (e.g. checking for specification expressions
>> while taking additionally the given constraints into account.)
>>     
> Well, it's actually not checking for specification expressions (which
> is what gfc_specification_expr does), but it checks for constant
> expressions, which are defined in chapter 7.1.12 of F08.
>   

No, it does not. A Fortran 2008 constant expression is a Fortran 2003
initialization expression and for array bounds one does not check for
F2008 constant/F2003 initialization expressions otherwise automatic
arrays would not be possible.

Seemingly, the gfortran names are based on Fortran 95 - thus looking at
F2003/F2008 does not help if one want to know what it meant. I think we
urgently need to add in the comment a reference to the relevant sections
(and terms) of all three standards - otherwise, one gets completely
confused.

That's seems to be complicated by the problem that Fortran 95 seems
define have four terms while Fortran 2003 and Fortran 2008 define only
three terms. In particular, F95 has constant and initialization
expressions, F2003 has initialization expressions, and F2008 has
constant expressions. I think I will print out the few pages of the
three standard and try to get an idea how the terms relate in the
different standards.


Fortran 95 has:

* [7.1.6.1] A *constant expression* is an expression in which each
operation is intrinsic and each primary is [...]

* [7.1.6.1] An *initialization expression* is a constant expression in
which the exponentiation operation is
permitted only with an integer power, and each primary is [...]

* [7.1.6.2] A *specification expression* is an expression with
limitations that make it suitable for use in
specifications such as character lengths (R510) and array bounds (R515,
R516). A constant
specification expression is a specification expression that is also a
constant expression.

* [7.1.6.2] A *restricted expression* is an expression in which each
operation is intrinsic and each primary is [...]


Fortran 2003 has:

* [7.1.6] A *specification expression* is an expression with limitations
that make it suitable for use in specifications such as length type
parameters (C501) and array bounds (R512, R513). [....]

* [7.1.6] A *restricted expression* is an expression in which each
operation is intrinsic and each primary is [...]

* [7.1.7] An *initialization expression* is an expression with
limitations that make it suitable for use as a kind type parameter,
initializer, or named constant. It is an expression in which each
operation is intrinsic, and each primary is [...]


Fortran 2008 has:

* [7.1.11] A *specification expression* is an expression with
limitations that make it suitable for use in specifications such as
length type parameters (C404) and array bounds (R517, R518). A
specification-expr shall be a constant expression unless it is in an
interface body (12.4.3.2), the specification part of a subprogram or
BLOCK construct, a derived-type definition, or the declaration-type-spec
of a FUNCTION statement (12.6.2.2). [...]

* [7.1.11] A *restricted expression* is an expression in which each
operation is intrinsic or de\fned by a specification function and each
primary is [...]

* [7.1.12] A *constant expression* is an expression with limitations
that make it suitable for use as a kind type parameter, initializer, or
named constant. It is an expression in which each operation is
intrinsic, and each primary is [...]


And gfortran has:

* gfc_is_constant_expr: Function to determine if an expression is
constant or not.  This function expects that the expression has already
been simplified.

* check_restricted: Verify that an expression is a restricted
expression.  Like its cousin check_init_expr(), an error message is
generated if we return FAILURE.

* check_init_expr: Verify that an expression is an initialization
expression. [...]

* gfc_specification_expr: Check to see that an expression is a
specification expression. If we return FAILURE, an error has been generated.


Tobias

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 2030 bytes --]

2010/7/26 Tobias Burnus <burnus@net-b.de>:
> Janus Weil wrote:
>>> Can you add a comment before the gfc_is_constant_expr to state what the
>>> function is actually doing (e.g. checking for specification expressions
>>> while taking additionally the given constraints into account.)
>>>
>> Well, it's actually not checking for specification expressions (which
>> is what gfc_specification_expr does), but it checks for constant
>> expressions, which are defined in chapter 7.1.12 of F08.
>>
>
> No, it does not. A Fortran 2008 constant expression is a Fortran 2003
> initialization expression and for array bounds one does not check for
> F2008 constant/F2003 initialization expressions otherwise automatic
> arrays would not be possible.
>
> Seemingly, the gfortran names are based on Fortran 95 - thus looking at
> F2003/F2008 does not help if one want to know what it meant. I think we
> urgently need to add in the comment a reference to the relevant sections
> (and terms) of all three standards - otherwise, one gets completely
> confused.
>
> That's seems to be complicated by the problem that Fortran 95 seems
> define have four terms while Fortran 2003 and Fortran 2008 define only
> three terms. In particular, F95 has constant and initialization
> expressions, F2003 has initialization expressions, and F2008 has
> constant expressions. I think I will print out the few pages of the
> three standard and try to get an idea how the terms relate in the
> different standards.

Ok, fine. Apparently I have trouble understanding the subtleties of
how different versions of the standard use these terms in different
ways.

I guess what I'll do is just commit the the one hunk which fixes the
original problem (which has been approved already) and leave all the
rest alone.

Cheers,
Janus


2010-07-29  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/44962
	* resolve.c (resolve_fl_derived): Call gfc_resolve_array_spec.


2010-07-29  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/44962
	* gfortran.dg/typebound_proc_17.f03: New.

[-- Attachment #2: pr44962_v5.diff --]
[-- Type: application/octet-stream, Size: 1325 bytes --]

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 162687)
+++ gcc/fortran/resolve.c	(working copy)
@@ -10813,7 +10813,6 @@ resolve_fl_derived (gfc_symbol *sym)
 {
   gfc_symbol* super_type;
   gfc_component *c;
-  int i;
 
   super_type = gfc_get_derived_super_type (sym);
   
@@ -11169,25 +11168,10 @@ resolve_fl_derived (gfc_symbol *sym)
 	    && sym != c->ts.u.derived)
 	add_dt_to_dt_list (c->ts.u.derived);
 
-      if (c->attr.pointer || c->attr.proc_pointer || c->attr.allocatable
-	  || c->as == NULL)
-	continue;
-
-      for (i = 0; i < c->as->rank; i++)
-	{
-	  if (c->as->lower[i] == NULL
-	      || (resolve_index_expr (c->as->lower[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->lower[i])
-	      || c->as->upper[i] == NULL
-	      || (resolve_index_expr (c->as->upper[i]) == FAILURE)
-	      || !gfc_is_constant_expr (c->as->upper[i]))
-	    {
-	      gfc_error ("Component '%s' of '%s' at %L must have "
-			 "constant array bounds",
-			 c->name, sym->name, &c->loc);
-	      return FAILURE;
-	    }
-	}
+      if (gfc_resolve_array_spec (c->as, !(c->attr.pointer
+					   || c->attr.proc_pointer
+					   || c->attr.allocatable)) == FAILURE)
+	return FAILURE;
     }
 
   /* Resolve the type-bound procedures.  */

[-- Attachment #3: typebound_proc_17.f03 --]
[-- Type: application/octet-stream, Size: 499 bytes --]

! { dg-do compile }
!
! PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)
!
! Contributed by Satish.BD <bdsatish@gmail.com>


module array

type :: t_array
  real, dimension(10) :: coeff
contains
  procedure :: get_coeff
end type t_array

contains

function get_coeff(self) result(coeff)
  class(t_array), intent(in) :: self
  real, dimension(size(self%coeff)) :: coeff !! The SIZE here carashes !!
end function get_coeff

end module array


! { dg-final { cleanup-modules "array" } }

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Janus Weil]

> I guess what I'll do is just commit the the one hunk which fixes the
> original problem (which has been approved already) and leave all the
> rest alone.

Committed as r162695.

Cheers,
Janus



> 2010-07-29  Janus Weil  <janus@gcc.gnu.org>
>
>        PR fortran/44962
>        * resolve.c (resolve_fl_derived): Call gfc_resolve_array_spec.
>
>
> 2010-07-29  Janus Weil  <janus@gcc.gnu.org>
>
>        PR fortran/44962
>        * gfortran.dg/typebound_proc_17.f03: New.
>

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Dominique Dhumieres]

Janus,

I just noticed that you have changed the test typebound_proc_17.f03
between revisions of your patch:

--- typebound_proc_17_old.f03	2010-07-26 07:45:13.000000000 +0200
+++ /opt/gcc/work/gcc/testsuite/gfortran.dg/typebound_proc_17.f03	2010-07-26 07:15:54.000000000 +0200
@@ -24,14 +24,11 @@ end module array
 
 
 type :: t2
-  real, dimension(irand()+5) :: co2  ! { dg-error "must be constant" }
+  real, dimension(iabs(-3)+2) :: com
 end type
 
-type(t2) :: a
+real, dimension(irand()+2) :: r2   ! { dg-error "must have constant shape" }
 
-print *, size(a%co2)
-a%co2 = 1.0
-print *, a
 end
 
 ! { dg-final { cleanup-modules "array" } }

(where I have added the last 4 '-' lines for testing as commented below).

With the latest patch the line

  real, dimension(irand()+5) :: co2  ! { dg-error "must be constant" }

in "type :: t2" no longer triggers the error. Is it according the last lines
of f2008 [7.1.11] quoted by Tobias Burnus? If yes, is it normal that
"print *, size(a%co2)" yields 0?

Also when compiled with -std=f2003, both versions give the error:

real, dimension(irand()+*) :: *   ! { dg-error "must *" }
                1
Error: Specification function 'irand' at (1) must be PURE

Has this constraint been removed in f2008?

Finally, as I said in a previous post, the *rand functions are
g77 extensions and their use should probably discouraged in
post f90 codes. If the issues raised by Tobias appear only
for these functions, is it really necessary to spend too
much time on them?

Cheers,

Dominique

Re: [Patch, Fortran, OOP] PR 44962: [OOP] ICE with specification expression SIZE(<CLASS>)

[Dominique Dhumieres]

If I have read correctly the gfortran manual, RAND and IRAND are GNU
extensions for compatibility with g77. Is it worth any effort to
include them in post f90 use?

Dominique