diff mbox

[Fortran,F08] PR45290: pointer initialization

Message ID AANLkTikyALG1nNXJoPdtW8gmeK0mKi-EYM30O22vjFbF@mail.gmail.com
State New
Headers show

Commit Message

Janus Weil Aug. 17, 2010, 3 p.m. UTC
Hi Tobias,

thanks for your feedback.

>> I hope everything should work as advertised. Regtesting was successful
>> on x86_64-unknown-linux-gnu (except for the continuing failure of
>> array_memcpy_3.f90, cf. PR45266).
>> Ok for trunk?
>
> For pointer initialization there is a case where the changed SAVE behaviour
> in Fortran 2008 matters. So far I had the impression that there is no such
> case, but seemingly we have now one. (Actually, that's not quite true: it
> also occurs for coarrays where I seem to handle it explicitly.)
>
> "A variable, common block, or procedure pointer declared in the scoping unit
> of a main program, module, or submodule implicitly has the SAVE attribute,
> which may be con rmed by explicit speci cation" (Fortran 2008, 5.3.16 SAVE
> attribute)
>
> Thus, I believe the following program is valid and should not be rejected.
> We have now two possibilities: (a) setting SAVE_IMPLICIT or (b) adding
> explicit check for pointer initialization.
>
>
> module m
>  integer, target  :: t1
>  integer, pointer :: p1 => t1 ! valid, "t1" is implicitly SAVE
> end module m

Ok, I have picked your option (a) and set SAVE_IMPLICIT in decl.c
(match_attr_spec). I also had to modify gfc_add_save to handle
SAVE_IMPLICIT (important e.g. when copying attributes).



> The following program ICEs (segfault) via
>    by 0x573B54: gfc_create_module_variable (trans-decl.c:3597)
> in
>    at 0x57E731: gfc_conv_variable (trans-expr.c:593)
>
>
> module m
>  integer, target, save  :: t1
>  integer, pointer :: p1 => t1
> end module m
>
> program main
>  use m
> end program main

Oops. To get rid of this I had to add the following hunk in trans-expr.c:

@@ -590,7 +590,8 @@ gfc_conv_variable (gfc_se * se, gfc_expr * expr)
       entry_master = sym->attr.result
 		     && sym->ns->proc_name->attr.entry_master
 		     && !gfc_return_by_reference (sym->ns->proc_name);
-      parent_decl = DECL_CONTEXT (current_function_decl);
+      if (current_function_decl)
+	parent_decl = DECL_CONTEXT (current_function_decl);

       if ((se->expr == parent_decl && return_value)
 	   || (sym->ns && sym->ns->proc_name

and then then another one:



> 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?


New patch attached.

Cheers,
Janus

Comments

Tobias Burnus Aug. 17, 2010, 8:36 p.m. UTC | #1
Hi Janus,

Janus Weil wrote:
>> 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)

I think the cleanest would be to remove the line and update the uses to 
include a "|| attr.pointer" or "|| attr.proc_pointer" where needed. -- 
And at the same time to remove the no longer needed && !attr.target). 
However, it might be a bit more lengthy task as one has to check several 
files.

Regarding the pointer initialization: In Fortran, a POINTER points to a 
named or anonymous target; thus, a pointer can never point to another 
pointer and "ptr2 => ptr1" associates the target of ptr1 with ptr2.  
Thus, "ptr2 => ptr1" does not make sense as initialization expression. 
Thus, also from this point of view it should be invalid.


Regarding your patch:

a) You do not set the implicit SAVE for the main program ("declared in 
the scoping unit of a main program, module, or submodule implicitly has 
the SAVE attribute") thus:

program main
   integer, target  :: t2
   integer, pointer :: p3 => t2
end program main

fails with:

Error: Pointer initialization target at (1) must have the SAVE attribute


b) The following gives still an ICE, but I probably should not be 
surprised because of the attr.target issue discussed above:

module m
   integer, target  :: 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

Otherwise, the patch looks OK to me.

Tobias
diff mbox

Patch

Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c	(revision 163296)
+++ gcc/fortran/trans-decl.c	(working copy)
@@ -3587,7 +3587,7 @@  gfc_create_module_variable (gfc_symbol * sym)
       && (sym->equiv_built || sym->attr.in_equivalence))
     return;

-  if (sym->backend_decl && !sym->attr.vtab)
+  if (sym->backend_decl && !sym->attr.vtab && !sym->attr.target)
     internal_error ("backend decl for module variable %s already exists",
 		    sym->name);