diff mbox

[Fortran,F08] PR45290: pointer initialization

Message ID AANLkTimEkr96PqN04QwDpxQfoKrg-FnuSa9oO1-z1cPJ@mail.gmail.com
State New
Headers show

Commit Message

Janus Weil Aug. 17, 2010, 9:19 p.m. UTC
>>> And the following is invalid and gives an ICE:
>>>
>>> module m
>>>  integer, target, save  :: t1
>>>  integer, pointer :: p1 =>  t1
>>>  integer, pointer, save :: p2 =>  p2 ! invalid&  ICE
>>>  integer, pointer :: p3 =>  p1 ! ICE&  invalid as "p1" is not a TARGET
>>> end module m
>>
>> About this one I'm still not sure. F08 explicitly says:
>>
>> C556 An entity with the TARGET attribute shall not have the POINTER
>> attribute.
>>
>> But still gfc_variable_attr seems to set the TARGET attribute for
>> things that actually are POINTERS. Can someone explain this?
>
> Well, it gains the attribute after the declaration part when it appears as
> expression: in primary.c's gfc_variable_attr. If one looks at svn blame, one
> sees that the following line exists from the beginning of GCC 4.0.0:
>  if (pointer || attr.proc_pointer)
>    target = 1;
>
> I assume the idea was to allow for checks such as:
>  if (RHS->attr.target)
>    return Pointer_association_is_allowed
> instead of needing to use
>  if (RHS->attr.target || RHS->attr.pointer)
>    return Pointer_association_is_allowed
>
> But in my opinion, that's highly misleading and currently requires to write
> code such as
>  (attr.target && !attr.pointer)


Moreover it's plain wrong. But adding this patchlet:



gives me a couple of regressions:


FAIL: gfortran.fortran-torture/execute/ptr.f90 compilation,  -O0
FAIL: gfortran.dg/c_loc_tests_14.f90  -O  (test for excess errors)
FAIL: gfortran.dg/c_loc_tests_5.f03  -O  (test for excess errors)
FAIL: gfortran.dg/pointer_assign_4.f90  -O0  (test for excess errors)
FAIL: gfortran.dg/pr43984.f90  -O  (test for excess errors)
FAIL: gfortran.dg/subref_array_pointer_1.f90  -O0  (test for excess errors)

All of them, except the C_LOC ones, fail on pointer assignments. And I
think all of them are actually invalid.



Some applicable quotes from F08:


Chapter 5.3.14:
 * C546 An entity with the POINTER attribute shall not have the
ALLOCATABLE, INTRINSIC, or TARGET attribute, and shall not be a
coarray.

Chapter 5.3.17:
 * C556 An entity with the TARGET attribute shall not have the POINTER
attribute.
 * If an object has the TARGET attribute, then all of its nonpointer
subobjects also have the TARGET attribute.

Chapter 6.4.2:
 * A structure component is a pointer only if the rightmost part name
is defined to have the POINTER attribute.

Chapter 6.5.3:
 * NOTE 6.10: Unless otherwise specified, an array element or array
section does not have an attribute of the whole array. In particular,
an array element or an array section does not have the POINTER or
ALLOCATABLE attribute.


Cheers,
Janus

Comments

Janus Weil Aug. 17, 2010, 9:48 p.m. UTC | #1
> But adding this patchlet:
>
>
> Index: gcc/fortran/primary.c
> ===================================================================
> --- gcc/fortran/primary.c       (revision 163310)
> +++ gcc/fortran/primary.c       (working copy)
> @@ -2017,8 +2017,6 @@ gfc_variable_attr (gfc_expr *expr, gfc_typespec *t
>     }
>
>   target = attr.target;
> -  if (pointer || attr.proc_pointer)
> -    target = 1;
>
>   if (ts != NULL && expr->ts.type == BT_UNKNOWN)
>     *ts = sym->ts;
> @@ -2074,8 +2072,6 @@ gfc_variable_attr (gfc_expr *expr, gfc_typespec *t
>            pointer = comp->attr.pointer;
>            allocatable = comp->attr.allocatable;
>          }
> -       if (pointer || attr.proc_pointer)
> -         target = 1;
>
>        break;
>
>
> gives me a couple of regressions:
>
>
> FAIL: gfortran.fortran-torture/execute/ptr.f90 compilation,  -O0
> FAIL: gfortran.dg/c_loc_tests_14.f90  -O  (test for excess errors)
> FAIL: gfortran.dg/c_loc_tests_5.f03  -O  (test for excess errors)
> FAIL: gfortran.dg/pointer_assign_4.f90  -O0  (test for excess errors)
> FAIL: gfortran.dg/pr43984.f90  -O  (test for excess errors)
> FAIL: gfortran.dg/subref_array_pointer_1.f90  -O0  (test for excess errors)
>
> All of them, except the C_LOC ones, fail on pointer assignments. And I
> think all of them are actually invalid.


Here is an updated patch, which fixes the invalid test cases. It
should be free of regressions.

Ok so far?

I will re-check for regressions and take care of implicit SAVE in
PROGRAMS tomorrow.

Cheers,
Janus
Tobias Burnus Aug. 17, 2010, 10:55 p.m. UTC | #2
Janus Weil wrote:
> FAIL: gfortran.fortran-torture/execute/ptr.f90 compilation,  -O0
> FAIL: gfortran.dg/c_loc_tests_14.f90  -O  (test for excess errors)
> FAIL: gfortran.dg/c_loc_tests_5.f03  -O  (test for excess errors)
> FAIL: gfortran.dg/pointer_assign_4.f90  -O0  (test for excess errors)
> FAIL: gfortran.dg/pr43984.f90  -O  (test for excess errors)
> FAIL: gfortran.dg/subref_array_pointer_1.f90  -O0  (test for excess errors)
>
> All of them, except the C_LOC ones, fail on pointer assignments. And I
> think all of them are actually invalid.

At least NAG f95 accepts them all.


+++ gcc/testsuite/gfortran.dg/pr43984.f90 (working copy)

-  real(kind=kind(1.0d0)), dimension(:), pointer :: Izz
-      Izz =>  Iz(:,z)


That looks perfectly valid, cf. below.

-    integer(c_int), dimension(:), pointer :: int_ptr
-    my_c_ptr = c_loc(int_ptr(0))


Well, as written is is invalid - but change it to

-    integer(c_int), dimension(:), pointer :: int_ptr
ALLOCATE(int_ptr(0:10))
-    my_c_ptr = c_loc(int_ptr(0))


Then it is valid. Note: "int_ptr(0)" is not a pointer but "int_ptr(0)" 
is the first element of the array to which int_ptr points. That array is 
unnamed but has the TARGET attribute. If you want to have a named 
target, use:

integer, target :: tg(0:10)
-    integer(c_int), dimension(:), pointer :: int_ptr
int_ptr =>  tg
-    my_c_ptr = c_loc(int_ptr(0))

In this case int_ptr(0) is the first element of "tg" and "tg" has the 
TARGET attribute.

Tobias
Janus Weil Aug. 18, 2010, 7:26 a.m. UTC | #3
> That looks perfectly valid, cf. below.
>
> -    integer(c_int), dimension(:), pointer :: int_ptr
> -    my_c_ptr = c_loc(int_ptr(0))
>
>
> Well, as written is is invalid - but change it to
>
> -    integer(c_int), dimension(:), pointer :: int_ptr
> ALLOCATE(int_ptr(0:10))
> -    my_c_ptr = c_loc(int_ptr(0))
>
>
> Then it is valid.


... which means that a through check for validity is very hard to do
at compile time, since it depends on the run-time value, right?



> Note: "int_ptr(0)" is not a pointer but "int_ptr(0)" is
> the first element of the array to which int_ptr points. That array is
> unnamed but has the TARGET attribute. If you want to have a named target,
> use:
>
> integer, target :: tg(0:10)
> -    integer(c_int), dimension(:), pointer :: int_ptr
> int_ptr =>  tg
> -    my_c_ptr = c_loc(int_ptr(0))
>
> In this case int_ptr(0) is the first element of "tg" and "tg" has the TARGET
> attribute.

Well, ok. I guess that is one way to look at it. However, if I apply
the same logic to your earlier pointer-init example ...


module m
 integer, target, save  :: t1
 integer, pointer :: p1 => t1
 integer, pointer :: p3 => p1
end module m

... then I'd say this is valid, too. p1 itself is a pointer, but the
thing that it points to is a target (namely t1). Therefore "p3 => p1"
is valid, since the object on the RHS has the TARGET attribute. Can we
agree on that?

Cheers,
Janus
diff mbox

Patch

Index: gcc/fortran/primary.c
===================================================================
--- gcc/fortran/primary.c	(revision 163310)
+++ gcc/fortran/primary.c	(working copy)
@@ -2017,8 +2017,6 @@  gfc_variable_attr (gfc_expr *expr, gfc_typespec *t
     }

   target = attr.target;
-  if (pointer || attr.proc_pointer)
-    target = 1;

   if (ts != NULL && expr->ts.type == BT_UNKNOWN)
     *ts = sym->ts;
@@ -2074,8 +2072,6 @@  gfc_variable_attr (gfc_expr *expr, gfc_typespec *t
 	    pointer = comp->attr.pointer;
 	    allocatable = comp->attr.allocatable;
 	  }
-	if (pointer || attr.proc_pointer)
-	  target = 1;

 	break;