[patch,wip] warn on noncontiguous pointers

[Cesar Philippidis]

As of GCC 8, gfortran now errors when a pointer with a contiguous
attribute is set to point to a target without a contiguous attribute. I
think this is overly strict, and should probably be demoted to a
pedantic warning as I've done in the attached patch.

I ran into this issue while I was tuning GCC for lsdalton. Specifically,
CMake generates (not exactly because I reduced it) the following test
case for ScaTeLib to determine if that library can be enabled:

program test
implicit none
real,pointer :: fptr1(:)
real,pointer,contiguous :: fptr3(:,:,:)

allocate(fptr1(12))
call random_number(fptr1)

!Test pointer reshape II

fptr3(1:2,1:2,1:2) => fptr1(4:)
end program

Note how fptr1 doesn't have a contiguous attribute. Does anyone have
thoughts on this? Maybe the ScaTeLib code needs to be updated.

Thanks,
Cesar

[Thomas Koenig]

Hi Cesar,

Post by Cesar Philippidis
As of GCC 8, gfortran now errors when a pointer with a contiguous
attribute is set to point to a target without a contiguous attribute. I
think this is overly strict, and should probably be demoted to a
pedantic warning as I've done in the attached patch.

We had a lengthy discussion on that one. Still, we can dig into the
standard for that one.

J3/10-007 says in 7.2.2.3 Data pointer assignment

# 7 If the pointer object has the CONTIGUOUS attribute, the pointer
# target shall be contiguous.

# 9 If bounds-remapping-list is specified, the pointer target shall
# be simply contiguous (6.5.4) or of rank one

program test
implicit none
real,pointer :: fptr1(:)
real,pointer,contiguous :: fptr3(:,:,:)

allocate(fptr1(12))
call random_number(fptr1)

!Test pointer reshape II

fptr3(1:2,1:2,1:2) => fptr1(4:)

end program

So, by paragraph 9, this would be OK. Let's see what paragraph 7
means when it says "contiguous". 5.3.7 says

An object is contiguous if it is

# (1) an object with the CONTIGUOUS attribute,
# (2) a nonpointer whole array that is not assumed-shape,
# (3) an assumed-shape array that is argument associated with an
array that is contiguous,
# (4) an array allocated by an ALLOCATE statement,
# (5) a pointer associated with a contiguous target, or
# (6) a nonzero-sized array section (6.5.3) provided that
# (a) its base object is contiguous,
# (b) it does not have a vector subscript,
# (c) the elements of the section, in array element order, are a
# subset of the base object elements that are consecutive in
# array element order,
# (d) if the array is of type character and a substring-range appears,
# the substring-range specifies all of the characters of the
# parent string (6.4.1),
# (e) only its final part-ref has nonzero rank, and
# (f) it is not the real or imaginary part (6.4.4) of an array of type
# complex.

An object is not contiguous if it is an array subobject, and

[conditions not relevant elided]

# It is processor dependent whether any other object is contiguous.

If we go down the list, we see that fptr1(4:) is not contiguous; it
is not an array (it is a pointer), so (4) also does not apply.

So, we are in the realm of processor dependent behavior, so we can
chose what to do.

The last time we discussed this, we agreed on a hard error. One
important argument is that a mistakenly applied contiguous
attribute will lead to wrong code, and that it is quite easy
to check this, as we do now.

So, I think we should leave the behavior as it is now, and

Post by Cesar Philippidis
Maybe the ScaTeLib code needs to be updated.

sounds like a good idea to me.

Regards

Thomas

[Cesar Philippidis]

Post by Thomas Koenig
Hi Cesar,

Post by Cesar Philippidis
As of GCC 8, gfortran now errors when a pointer with a contiguous
attribute is set to point to a target without a contiguous attribute. I
think this is overly strict, and should probably be demoted to a
pedantic warning as I've done in the attached patch.

We had a lengthy discussion on that one. Still, we can dig into the
standard for that one.
J3/10-007 says in 7.2.2.3  Data pointer assignment
# 7 If the pointer object has the CONTIGUOUS attribute, the pointer
# target shall be contiguous.
# 9 If bounds-remapping-list is specified, the pointer target shall
# be simply contiguous (6.5.4) or of rank one
program test
   implicit none
   real,pointer :: fptr1(:)
   real,pointer,contiguous :: fptr3(:,:,:)
   allocate(fptr1(12))
   call random_number(fptr1)
   !Test pointer reshape II
   fptr3(1:2,1:2,1:2) => fptr1(4:)
end program
So, by paragraph 9, this would be OK. Let's see what paragraph 7
means when it says "contiguous". 5.3.7 says
An object is contiguous if it is
# (1) an object with the CONTIGUOUS attribute,
# (2) a nonpointer whole array that is not assumed-shape,
# (3) an assumed-shape array that is argument associated with an
     array that is contiguous,
# (4) an array allocated by an ALLOCATE statement,
# (5) a pointer associated with a contiguous target, or
# (6) a nonzero-sized array section (6.5.3) provided that
#   (a) its base object is contiguous,
#   (b) it does not have a vector subscript,
#   (c) the elements of the section, in array element order, are a
#       subset of the base object elements that are consecutive in
#       array element order,
#   (d) if the array is of type character and a substring-range appears,
#       the substring-range specifies all of the characters of the
#       parent string (6.4.1),
#   (e) only its final part-ref has nonzero rank, and
#   (f) it is not the real or imaginary part (6.4.4) of an array of type
#       complex.
An object is not contiguous if it is an array subobject, and
[conditions not relevant elided]
# It is processor dependent whether any other object is contiguous.
If we go down the list, we see that fptr1(4:) is not contiguous; it
is not an array (it is a pointer), so (4) also does not apply.
So, we are in the realm of processor dependent behavior, so we can
chose what to do.
The last time we discussed this, we agreed on a hard error.  One
important argument is that a mistakenly applied contiguous
attribute will lead to wrong code, and that it is quite easy
to check this, as we do now.
So, I think we should leave the behavior as it is now, and

Thank you for the explanation. That all seems reasonable.

Post by Thomas Koenig

Post by Cesar Philippidis
Maybe the ScaTeLib code needs to be updated.

sounds like a good idea to me.

ACK.

Thanks,
Cesar

[Bader, Reinhold]

Hello all,

Post by Thomas Koenig
We had a lengthy discussion on that one. Still, we can dig into the
standard for that one.
J3/10-007 says in 7.2.2.3 Data pointer assignment
# 7 If the pointer object has the CONTIGUOUS attribute, the pointer
# target shall be contiguous.
# 9 If bounds-remapping-list is specified, the pointer target shall
# be simply contiguous (6.5.4) or of rank one
program test
implicit none
real,pointer :: fptr1(:)
real,pointer,contiguous :: fptr3(:,:,:)
allocate(fptr1(12))
call random_number(fptr1)
!Test pointer reshape II
fptr3(1:2,1:2,1:2) => fptr1(4:)
end program
So, by paragraph 9, this would be OK. Let's see what paragraph 7
means when it says "contiguous". 5.3.7 says
An object is contiguous if it is
# (1) an object with the CONTIGUOUS attribute,
# (2) a nonpointer whole array that is not assumed-shape,
# (3) an assumed-shape array that is argument associated with an
array that is contiguous,
# (4) an array allocated by an ALLOCATE statement,
# (5) a pointer associated with a contiguous target, or
# (6) a nonzero-sized array section (6.5.3) provided that
# (a) its base object is contiguous,
# (b) it does not have a vector subscript,
# (c) the elements of the section, in array element order, are a
# subset of the base object elements that are consecutive in
# array element order,
# (d) if the array is of type character and a substring-range appears,
# the substring-range specifies all of the characters of the
# parent string (6.4.1),
# (e) only its final part-ref has nonzero rank, and
# (f) it is not the real or imaginary part (6.4.4) of an array of type
# complex.
An object is not contiguous if it is an array subobject, and
[conditions not relevant elided]
# It is processor dependent whether any other object is contiguous.
If we go down the list, we see that fptr1(4:) is not contiguous; it
is not an array (it is a pointer), so (4) also does not apply.

I don't agree with this conclusion. First, the array and pointer properties
are not mutually exclusive (they are conveyed by specifying the DIMENSION
and
POINTER attribute, independently).
Specifically for the array section fptr1(4:) all conditions
in item (6) are fulfilled, and therefore it is a contiguous TARGET suitable
for appearing on the RHS of a contiguous pointer assignment.

Regards
Reinhold

Post by Thomas Koenig
So, we are in the realm of processor dependent behavior, so we can
chose what to do.
The last time we discussed this, we agreed on a hard error. One
important argument is that a mistakenly applied contiguous
attribute will lead to wrong code, and that it is quite easy
to check this, as we do now.
So, I think we should leave the behavior as it is now, and

Post by Cesar Philippidis
Maybe the ScaTeLib code needs to be updated.

sounds like a good idea to me.
Regards
Thomas

[Thomas Koenig]

Hi Reinhold,

Post by Bader, Reinhold

Post by Thomas Koenig
If we go down the list, we see that fptr1(4:) is not contiguous; it
is not an array (it is a pointer), so (4) also does not apply.

I don't agree with this conclusion. First, the array and pointer properties
are not mutually exclusive (they are conveyed by specifying the DIMENSION
and
POINTER attribute, independently).

Can you maybe elaborate a bit more?

I see that you can associate a pointer with a target
(6.7.1.4), which of course can be an array. But I have
not found anything in the standard that says that a
pointer to an array is an array.

Regards

Thomas

[Bader, Reinhold]

Hi Thomas,

-----Ursprüngliche Nachricht-----
Gesendet: Donnerstag, 27. September 2018 19:24
Betreff: Re: [patch,wip] warn on noncontiguous pointers
Hi Reinhold,

Post by Bader, Reinhold

Post by Thomas Koenig
If we go down the list, we see that fptr1(4:) is not contiguous; it
is not an array (it is a pointer), so (4) also does not apply.

I don't agree with this conclusion. First, the array and pointer
properties are not mutually exclusive (they are conveyed by specifying
the DIMENSION and POINTER attribute, independently).

Can you maybe elaborate a bit more?
I see that you can associate a pointer with a target (6.7.1.4), which of course
can be an array. But I have not found anything in the standard that says that a
pointer to an array is an array.

OK. Looking at 5.3.8.1 (Fortran 2008 for the sake of this discussion), we have that
an object that has the DIMENSION attribute is an array. array-spec includes deferred-shape-spec-list,
so any object declared with deferred shape is an array.

5.3.8.4 then says
C532 An array with the POINTER or ALLOCATABLE attribute shall have an array-spec that is a deferred-shape-spec-list.

and then has further text to say at what point things like SHAPE or SIZE of such an object become defined.

It can't get any clearer than that.

Cheers
Reinhold

Regards
Thomas

[N.M. Maclaren]

Post by Bader, Reinhold

Post by Thomas Koenig

Post by Bader, Reinhold

Post by Thomas Koenig
If we go down the list, we see that fptr1(4:) is not contiguous; it
is not an array (it is a pointer), so (4) also does not apply.

I don't agree with this conclusion. First, the array and pointer
properties are not mutually exclusive (they are conveyed by specifying
the DIMENSION and POINTER attribute, independently).

Can you maybe elaborate a bit more?
I see that you can associate a pointer with a target (6.7.1.4), which
of course can be an array. But I have not found anything in the standard
that says that a pointer to an array is an array.

OK. Looking at 5.3.8.1 (Fortran 2008 for the sake of this discussion), we
have that an object that has the DIMENSION attribute is an array.
array-spec includes deferred-shape-spec-list, so any object declared with
deferred shape is an array.
5.3.8.4 then says C532 An array with the POINTER or ALLOCATABLE attribute
shall have an array-spec that is a deferred-shape-spec-list.
and then has further text to say at what point things like SHAPE or SIZE
of such an object become defined.
It can't get any clearer than that.

My guess is that the root cause of the confusion is the phrasing 'pointer
to an array', because that implies a model in which the pointer is
separate. It's better called an array pointer, where the two properties are
inseparable (for that name), and the comment "it is not an array (it is a
pointer)" is not correct - it's both. Attributes are not qualifiers in the
C sense.

That's why several of us who teach Fortran tell C and C++ programmers that
they will first have to unlearn much of what they know - the language's
abstract model is extremely different!


Regards,
Nick Maclaren.

[Thomas Koenig]

Post by Bader, Reinhold
Looking at 5.3.8.1 (Fortran 2008 for the sake of this discussion), we have that
an object that has the DIMENSION attribute is an array. array-spec includes deferred-shape-spec-list,
so any object declared with deferred shape is an array.

OK, I see, so this code is indeed legal. Thanks!

@Cesar: The idea behind your patch is sound. I think this would
fit into the "legal, but can sometimes bite you" category,
so I think would be better with -Wextra (where we accept that
there might be quite a few false positives).

Could you adapt your patch accordingly and also add two test cases,
one testing for the absence of the warning/error with normal
options, and one for the warning with -Wextra?

Regards

Thomas

[Cesar Philippidis]

  Looking at 5.3.8.1 (Fortran 2008 for the sake of this discussion),
we have that
an object that has the DIMENSION attribute is an array. array-spec
includes deferred-shape-spec-list,
so any object declared with deferred shape is an array.

OK, I see, so this code is indeed legal.  Thanks!
@Cesar:  The idea behind your patch is sound.  I think this would
fit into the "legal, but can sometimes bite you" category,
so I think would be better with -Wextra (where we accept that
there might be quite a few false positives).
Could you adapt your patch accordingly and also add two test cases,
one testing for the absence of the warning/error with normal
options, and one for the warning with -Wextra?

Yes, sure.

Thanks,
Cesar

[Cesar Philippidis]

  Looking at 5.3.8.1 (Fortran 2008 for the sake of this discussion),
we have that
an object that has the DIMENSION attribute is an array. array-spec
includes deferred-shape-spec-list,
so any object declared with deferred shape is an array.

OK, I see, so this code is indeed legal.  Thanks!
@Cesar:  The idea behind your patch is sound.  I think this would
fit into the "legal, but can sometimes bite you" category,
so I think would be better with -Wextra (where we accept that
there might be quite a few false positives).
Could you adapt your patch accordingly and also add two test cases,
one testing for the absence of the warning/error with normal
options, and one for the warning with -Wextra?

How's this patch look? I bootstrapped and regression tested it for
x86_64 Linux with nvptx offloading (not that it exercised any offloading
functionality).

Is it OK for trunk?

Thanks,
Cesar

[Thomas Koenig]

Hi Cesar,

Post by Cesar Philippidis
How's this patch look? I bootstrapped and regression tested it for
x86_64 Linux with nvptx offloading (not that it exercised any offloading
functionality).
Is it OK for trunk?

OK.

Maybe we should one day revisit this one day and separate out the cases
where we can prove that the array cannot be contiguous vs. the ones
where we cannot prove either way.

For now, this fixes a rejects-valid.

Thanks!

Regards

Thomas