diff mbox series

[v2] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652]

Message ID aec53a51-4c11-d565-dc89-d6d63d3be0d8@gmx.de
State New, archived
Headers
Series [v2] Fortran: fix invalid rank error in ASSOCIATED when rank is remapped [PR77652] |

Commit Message

Harald Anlauf July 27, 2022, 7:45 p.m. UTC 
  Hi Mikael,

Am 26.07.22 um 21:25 schrieb Mikael Morin:
> Le 25/07/2022 à 22:18, Harald Anlauf a écrit :
>> I would normally trust NAG more than Intel and Cray.
> … and yourself, it seems.  Too bad.
>
>> If somebody else convinces me to accept that NAG has it wrong this
>> time, I would be happy to proceed.
> It won’t convince you about NAG, but here are two reasons to proceed:
>   - Consensus among the maintainers is sufficient; it’s the case here.
>   - If uncertain, let’s be rather too permissive than too strict; it’s
> fine as long as the runtime answer is right.

ok, I have thought about your comments in the review process,
and played with the Cray compiler.  Attached is a refined version
of the patch that now rejects in addition these cases for which there
are no possible related pointer assignments with bounds remapping:

   ASSOCIATED (scalar, array) ! impossible, cannot remap bounds
   ASSOCIATED (array, scalar) ! a scalar is not simply contiguous

(Cray would allow those two, but IMHO these should be disallowed).

See attached for version 2 with updated testcase, regtested again.

I think this is what we could both be happy with... ;-)

Thanks,
Harald

Comments

Toon Moene July 27, 2022, 7:50 p.m. UTC | #1 
On 7/27/22 21:45, Harald Anlauf via Fortran wrote:

> Hi Mikael,

> Am 26.07.22 um 21:25 schrieb Mikael Morin:

>> Le 25/07/2022 à 22:18, Harald Anlauf a écrit :

>>> I would normally trust NAG more than Intel and Cray. 
>> … and yourself, it seems.  Too bad.
May I suggest that, if well known Fortran compilers differ in the 
treatment of this case, there might be reason to ask the Fortran 
Standard Committee for an Interpretation of the Standard ?

Kind regards,
Mikael Morin July 28, 2022, 8:19 p.m. UTC | #2 
Hello,

Le 27/07/2022 à 21:45, Harald Anlauf via Fortran a écrit :
> ok, I have thought about your comments in the review process,
> and played with the Cray compiler.  Attached is a refined version
> of the patch that now rejects in addition these cases for which there
> are no possible related pointer assignments with bounds remapping:
> 
>    ASSOCIATED (scalar, array) ! impossible, cannot remap bounds
>    ASSOCIATED (array, scalar) ! a scalar is not simply contiguous
> 
In principle, it could make sense to construct a one-sized array pointer 
(of any rank) pointing to a scalar, but I agree that if we follow the 
rules of the standard to the letter, it should be rejected (and we do 
reject such a pointer assignment).
So, with this case eliminated, this patch looks good to me as is.

Regarding Toon’s suggestion to ask the fortran committee, it sounds 
sensible.  I propose to prepare something tomorrow.
Harald Anlauf July 29, 2022, 8:01 p.m. UTC | #3 
Am 28.07.22 um 22:19 schrieb Mikael Morin:
> Hello,
>
> Le 27/07/2022 à 21:45, Harald Anlauf via Fortran a écrit :
>> ok, I have thought about your comments in the review process,
>> and played with the Cray compiler.  Attached is a refined version
>> of the patch that now rejects in addition these cases for which there
>> are no possible related pointer assignments with bounds remapping:
>>
>>    ASSOCIATED (scalar, array) ! impossible, cannot remap bounds
>>    ASSOCIATED (array, scalar) ! a scalar is not simply contiguous
>>
> In principle, it could make sense to construct a one-sized array pointer
> (of any rank) pointing to a scalar, but I agree that if we follow the
> rules of the standard to the letter, it should be rejected (and we do
> reject such a pointer assignment).
> So, with this case eliminated, this patch looks good to me as is.

OK, so I will push that version soon.

> Regarding Toon’s suggestion to ask the fortran committee, it sounds
> sensible.  I propose to prepare something tomorrow.
>

Depending on the answer we can later refine the compile-time check
and relax if needed.

Harald
Harald Anlauf Aug. 18, 2022, 7:32 p.m. UTC | #4 
Hi Mikael, all,

I've just reverted commit 0110cfd5449bae3a772f45ea2e4c5dab5b7a8ccd.
As it seems that commit ca170ed9f8a086ca7e1eec841882b6bed9ec1a3a did
not update bugzilla, I'll add a note to the PR and close it as invalid.

Thanks,
Harald


Am 04.08.22 um 14:03 schrieb Mikael Morin:
> Le 30/07/2022 à 12:03, Mikael Morin a écrit :
>> Le 28/07/2022 à 22:19, Mikael Morin a écrit :
>>> I propose to prepare something tomorrow.
>>>
>>
>> Here you go.
>
> I posted the message the other day.
> The mailing list archive are not automatic, so there is no link to the
> message (yet?), nor to the thread that follows it.
> So I attach below the answer from Malcolm Cohen.
> Long story short, he confirms the interpretation from Steve Lionel, and
> that the text in the standard needs fixing.
> I’m afraid we’ll have to revert.
>
>
> -------- Message transféré --------
> Sujet : [SC22WG5.6416] RE: [ukfortran] Request for interpretation of
> compile-time restrictions on ASSOCIATED
> Date : Thu, 4 Aug 2022 11:43:16 +0900
> De : Malcolm Cohen <malcolm@nag-j.co.jp>
> Pour : 'Mikael Morin' <morin-mikael@orange.fr>, sc22wg5@open-std.org
> Copie à : 'Harald Anlauf' <anlauf@gmx.de>
>
> Dear Mikael,
>
> Thank you for your query.
>
> I would agree with Steve Lionel that the ranks must be the same (when
> POINTER is not assumed-rank), for two reasons.
>
> (1) The result of ASSOCIATED is unambiguously .FALSE. when the shapes of
> POINTER and TARGET differ. As the shapes cannot be the same when the ranks
> differ seeing as how the number of elements in the shape are not the same,
> that means it would always be .FALSE. when the ranks differ. The Fortran
> language does not need an extra way to produce the LOGICAL constant .FALSE.
>
> Note that this means that even in the case where POINTER is dimension (2,1)
> and TARGET is dimension (1,2), and they both refer to the same elements in
> array element order, ASSOCIATED will return .FALSE. because the shapes are
> not the same. ASSOCIATED is a much stronger test than mere address
> comparison.
>
> (2) This text arises from an attempted, but failed, simplification of what
> we had before. Unfortunately, it is completely and utterly broken, as it
> forbids the use of ASSOCIATED when POINTER is assumed-rank, has INTENT(IN),
> is PROTECTED (outside of its module), or is a pointer function reference.
> That is because there are no pointer assignment statements where the
> pointer
> object is permitted to be any of those, and thus the conditions for TARGET
> cannot ever be satisfied.
>
> However, the processor is not *required* to report an error when the ranks
> differ, as this is not a "Constraint" in the standard. I would expect a
> high
> quality implementation to do so, but maybe I just have high expectations...
>
> It could also be a deliberate extension, with different semantics provided
> by the processor. In that case, the processor would be required to have the
> capability to report the use of the extension (but this need not be the
> default).
>
> Finally, I note that we are not accepting interpretation requests on
> Fortran
> 2018 at this time, as we are in the process of replacing it with a new
> revision (Fortran 2023). However, we will certainly consider whether we can
> make any correction to Fortran 2023 before publication (expected next
> year);
> if there is consensus on how to fix the clearly-incorrect requirements on
> TARGET, we can do so. Otherwise, we will need to wait until after Fortran
> 2023 is published before we can restart the Defect Processing process.
>
> I will undertake to write a meeting paper addressing this issue before this
> year's October meeting. If no paper has appeared by mid-September, please
> feel free to remind me to do that!
>
> Cheers,
diff mbox series

Patch

From 5432880ff21de862c64d79626aa19c4eda928cd5 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Wed, 27 Jul 2022 21:34:22 +0200
Subject: [PATCH] Fortran: fix invalid rank error in ASSOCIATED when rank is
 remapped [PR77652]

gcc/fortran/ChangeLog:

	PR fortran/77652
	* check.cc (gfc_check_associated): Make the rank check of POINTER
	vs. TARGET match the allowed forms of pointer assignment for the
	selected Fortran standard.

gcc/testsuite/ChangeLog:

	PR fortran/77652
	* gfortran.dg/associated_target_9a.f90: New test.
	* gfortran.dg/associated_target_9b.f90: New test.
---
 gcc/fortran/check.cc                          | 23 ++++++++++++++--
 .../gfortran.dg/associated_target_9a.f90      | 27 +++++++++++++++++++
 .../gfortran.dg/associated_target_9b.f90      | 23 ++++++++++++++++
 3 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/associated_target_9a.f90
 create mode 100644 gcc/testsuite/gfortran.dg/associated_target_9b.f90

diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
index 91d87a1b2c1..1da0b3cbe15 100644
--- a/gcc/fortran/check.cc
+++ b/gcc/fortran/check.cc
@@ -1502,8 +1502,27 @@  gfc_check_associated (gfc_expr *pointer, gfc_expr *target)
     t = false;
   /* F2018 C838 explicitly allows an assumed-rank variable as the first
      argument of intrinsic inquiry functions.  */
-  if (pointer->rank != -1 && !rank_check (target, 0, pointer->rank))
-    t = false;
+  if (pointer->rank != -1 && pointer->rank != target->rank)
+    {
+      if (pointer->rank == 0 || target->rank == 0)
+	{
+	  /* There exists no valid pointer assignment using bounds
+	     remapping for scalar => array or array => scalar. */
+	  if (!rank_check (target, 0, pointer->rank))
+	    t = false;
+	}
+      else if (target->rank != 1)
+	{
+	  if (!gfc_notify_std (GFC_STD_F2008, "Rank remapping target is not "
+			       "rank 1 at %L", &target->where))
+	    t = false;
+	}
+      else if ((gfc_option.allow_std & GFC_STD_F2003) == 0)
+	{
+	  if (!rank_check (target, 0, pointer->rank))
+	    t = false;
+	}
+    }
   if (target->rank > 0 && target->ref)
     {
       for (i = 0; i < target->rank; i++)
diff --git a/gcc/testsuite/gfortran.dg/associated_target_9a.f90 b/gcc/testsuite/gfortran.dg/associated_target_9a.f90
new file mode 100644
index 00000000000..708645d5bcb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/associated_target_9a.f90
@@ -0,0 +1,27 @@ 
+! { dg-do run }
+! { dg-options "-std=f2018" }
+! PR fortran/77652 - Invalid rank error in ASSOCIATED when rank is remapped
+! Contributed by Paul Thomas
+
+program p
+  real, dimension(100),  target  :: array
+  real, dimension(:,:),  pointer :: matrix
+  real, dimension(20,5), target  :: array2
+  real, dimension(:),    pointer :: matrix2
+  matrix(1:20,1:5) => array
+  matrix2(1:100)   => array2
+  !
+  ! F2018:16.9.16, ASSOCIATED (POINTER [, TARGET])
+  ! Case(v): If TARGET is present and is an array target, the result is
+  ! true if and only if POINTER is associated with a target that has
+  ! the same shape as TARGET, ...
+  if (associated (matrix, array )) stop 1
+  if (associated (matrix2,array2)) stop 2
+  call check (matrix2, array2)
+contains
+  subroutine check (ptr, tgt)
+    real, pointer :: ptr(..)
+    real, target  :: tgt(:,:)
+    if (associated (ptr, tgt)) stop 3
+  end subroutine check
+end
diff --git a/gcc/testsuite/gfortran.dg/associated_target_9b.f90 b/gcc/testsuite/gfortran.dg/associated_target_9b.f90
new file mode 100644
index 00000000000..1daa0a7dde1
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/associated_target_9b.f90
@@ -0,0 +1,23 @@ 
+! { dg-do compile }
+! { dg-options "-std=f2003" }
+! PR fortran/77652 - Invalid rank error in ASSOCIATED when rank is remapped
+! Contributed by Paul Thomas
+
+subroutine s
+  real, dimension(100),  target  :: array
+  real, dimension(:,:),  pointer :: matrix
+  real, dimension(20,5), target  :: array2
+  real, dimension(:),    pointer :: matrix2
+  real,                  pointer :: scalar, scalar2
+  scalar => scalar2
+  print *, associated (scalar, scalar2)
+
+  matrix(1:20,1:5) => array            ! F2003+
+! matrix2(1:100)   => array2           ! F2008+
+  print *, associated (matrix, array ) ! Technically legal F2003
+  print *, associated (matrix2,array2) ! { dg-error "is not rank 1" }
+
+  ! There exists no related valid pointer assignment for these cases:
+  print *, associated (scalar,matrix2) ! { dg-error "must be of rank 0" }
+  print *, associated (matrix2,scalar) ! { dg-error "must be of rank 1" }
+end
-- 
2.35.3