public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PR fortran/60091, patch] - Misleading error messages in rank-2 pointer assignment to rank-1 target
@ 2019-03-07 23:13 Harald Anlauf
  0 siblings, 0 replies; 7+ messages in thread
From: Harald Anlauf @ 2019-03-07 23:13 UTC (permalink / raw)
  To: gfortran, gcc-patches

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

The PR rightly complains about bad error messages for invalid pointer
assignments.  I've tried to adjust the logic slightly so that we now
print error messages that should explain more clearly what is wrong.

This required adjustment of 2 testcases, one of which also had an
incorrect comment.

OK for trunk?

Thanks,
Harald

2019-03-07  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/60091
	* expr.c (gfc_check_pointer_assign): Correct and improve error
	messages for invalid pointer assignments.

2019-03-07  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/60091
	* gfortran.dg/pointer_remapping_3.f08: Adjust error messages.
	* gfortran.dg/pointer_remapping_7.f90: Adjust error message.


[-- Attachment #2: patch-pr60091 --]
[-- Type: text/plain, Size: 3499 bytes --]

Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 269445)
+++ gcc/fortran/expr.c	(working copy)
@@ -3703,6 +3703,7 @@
   gfc_ref *ref;
   bool is_pure, is_implicit_pure, rank_remap;
   int proc_pointer;
+  bool same_rank;
 
   lhs_attr = gfc_expr_attr (lvalue);
   if (lvalue->ts.type == BT_UNKNOWN && !lhs_attr.proc_pointer)
@@ -3724,6 +3725,7 @@
   proc_pointer = lvalue->symtree->n.sym->attr.proc_pointer;
 
   rank_remap = false;
+  same_rank = lvalue->rank == rvalue->rank;
   for (ref = lvalue->ref; ref; ref = ref->next)
     {
       if (ref->type == REF_COMPONENT)
@@ -3748,36 +3750,67 @@
 			       lvalue->symtree->n.sym->name, &lvalue->where))
 	    return false;
 
-	  /* When bounds are given, all lbounds are necessary and either all
-	     or none of the upper bounds; no strides are allowed.  If the
-	     upper bounds are present, we may do rank remapping.  */
+	  /* Fortran standard (e.g. F2018, 10.2.2 Pointer assignment):
+	   *
+	   * (C1017) If bounds-spec-list is specified, the number of
+	   * bounds-specs shall equal the rank of data-pointer-object.
+	   *
+	   * If bounds-spec-list appears, it specifies the lower bounds.
+	   *
+	   * (C1018) If bounds-remapping-list is specified, the number of
+	   * bounds-remappings shall equal the rank of data-pointer-object.
+	   *
+	   * If bounds-remapping-list appears, it specifies the upper and
+	   * lower bounds of each dimension of the pointer; the pointer target
+	   * shall be simply contiguous or of rank one.
+	   *
+	   * (C1019) If bounds-remapping-list is not specified, the ranks of
+	   * data-pointer-object and data-target shall be the same.
+	   *
+	   * Thus when bounds are given, all lbounds are necessary and either
+	   * all or none of the upper bounds; no strides are allowed.  If the
+	   * upper bounds are present, we may do rank remapping.  */
 	  for (dim = 0; dim < ref->u.ar.dimen; ++dim)
 	    {
-	      if (!ref->u.ar.start[dim]
-		  || ref->u.ar.dimen_type[dim] != DIMEN_RANGE)
+	      if (ref->u.ar.stride[dim])
 		{
-		  gfc_error ("Lower bound has to be present at %L",
+		  gfc_error ("Stride must not be present at %L",
 			     &lvalue->where);
 		  return false;
 		}
-	      if (ref->u.ar.stride[dim])
+	      if (!same_rank && (!ref->u.ar.start[dim] ||!ref->u.ar.end[dim]))
 		{
-		  gfc_error ("Stride must not be present at %L",
+		  gfc_error ("Rank remapping requires a "
+			     "bounds-specification-list at %L",
 			     &lvalue->where);
 		  return false;
 		}
+	      if (!ref->u.ar.start[dim]
+		  || ref->u.ar.dimen_type[dim] != DIMEN_RANGE)
+		{
+		  gfc_error ("Expected bounds-remapping-list or "
+			     "bounds-specification-list at %L",
+			     &lvalue->where);
+		  return false;
+		}
 
 	      if (dim == 0)
 		rank_remap = (ref->u.ar.end[dim] != NULL);
 	      else
 		{
-		  if ((rank_remap && !ref->u.ar.end[dim])
-		      || (!rank_remap && ref->u.ar.end[dim]))
+		  if ((rank_remap && !ref->u.ar.end[dim]))
 		    {
-		      gfc_error ("Either all or none of the upper bounds"
-				 " must be specified at %L", &lvalue->where);
+		      gfc_error ("Rank remapping requires a "
+				 "bounds-specification-list at %L",
+				 &lvalue->where);
 		      return false;
 		    }
+		  if (!rank_remap && ref->u.ar.end[dim])
+		    {
+		      gfc_error ("Expected bounds-remapping-list or "
+				 "bounds-specification-list at %L",
+				 &lvalue->where);
+		    }
 		}
 	    }
 	}

[-- Attachment #3: patch-pr60091-testcases --]
[-- Type: text/plain, Size: 2005 bytes --]

Index: gcc/testsuite/gfortran.dg/pointer_remapping_3.f08
===================================================================
--- gcc/testsuite/gfortran.dg/pointer_remapping_3.f08	(revision 269445)
+++ gcc/testsuite/gfortran.dg/pointer_remapping_3.f08	(working copy)
@@ -3,6 +3,7 @@
 
 ! PR fortran/29785
 ! PR fortran/45016
+! PR fortran/60091
 ! Check for pointer remapping compile-time errors.
 
 ! Contributed by Daniel Kraft, d@domob.eu.
@@ -13,13 +14,13 @@
   INTEGER, POINTER :: vec(:), mat(:, :)
 
   ! Existence of reference elements.
-  vec(:) => arr ! { dg-error "Lower bound has to be present" }
-  vec(5:7:1) => arr ! { dg-error "Stride must not be present" }
-  mat(1:, 2:5) => arr ! { dg-error "Either all or none of the upper bounds" }
-  mat(2, 6) => arr ! { dg-error "Expected bounds specification" }
+  vec(:) => arr ! { dg-error "bounds-remapping-list or bounds-specification-list" }
+  vec(5:7:1)  => arr ! { dg-error "Stride must not be present" }
+  mat(1:,2:5) => arr ! { dg-error "requires a bounds-specification-list" }
+  mat(1:3,4:) => arr ! { dg-error "requires a bounds-specification-list" }
+  mat(2, 6)   => arr ! { dg-error "Expected bounds specification" }
 
-  ! This is bound remapping not rank remapping!
-  mat(1:, 3:) => arr ! { dg-error "Different ranks" }
+  mat(1:,3:)  => arr ! { dg-error "requires a bounds-specification-list" }
 
   ! Invalid remapping target; for non-rank one we already check the F2008
   ! error elsewhere.  Here, test that not-contiguous target is disallowed
Index: gcc/testsuite/gfortran.dg/pointer_remapping_7.f90
===================================================================
--- gcc/testsuite/gfortran.dg/pointer_remapping_7.f90	(revision 269445)
+++ gcc/testsuite/gfortran.dg/pointer_remapping_7.f90	(working copy)
@@ -4,5 +4,5 @@
 !
   integer, target :: A(100)
   integer,pointer :: P(:,:)
-  p(10,1:) => A  ! { dg-error "Lower bound has to be present" }
+  p(10,1:) => A  ! { dg-error "Expected bounds-remapping-list" }
   end

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR fortran/60091, patch] - Misleading error messages in rank-2 pointer assignment to rank-1 target
  2019-03-12 22:49   ` Thomas Koenig
  2019-03-13  0:39     ` Steve Kargl
  2019-03-13 13:01     ` Aw: " Harald Anlauf
@ 2019-03-15 22:33     ` Harald Anlauf
  2 siblings, 0 replies; 7+ messages in thread
From: Harald Anlauf @ 2019-03-15 22:33 UTC (permalink / raw)
  To: Thomas Koenig; +Cc: Dominique d'Humières, gfortran, gcc-patches

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

I've committed a slightly rewritten version of the error messages
to trunk as rev.269717, see attached.

Thanks for the review and the comments.

Harald

On 03/12/19 23:19, Thomas Koenig wrote:
> Hi Harald,
> 
>> how about the attached version?  It is quite verbose and produces
>> messages like
>>
>> Error: Expected list of 'lower-bound-expr:' or list of
>> 'lower-bound-expr:upper-bound-expr' at (1)
> 
> I think this way of specifying error messages
> 
> +#define BOUNDS_SPEC_LIST "list of %<lower-bound-expr:upper-bound-expr%>"
> 
> ...
> 
> +          gfc_error ("Rank remapping requires a "
> +                 BOUNDS_SPEC_LIST " at %L",
>                   &lvalue->where);
> 
> will cause trouble in translation of the error messages.
> 
> Could you maybe use something like
> 
> +          gfc_error ("Rank remapping requires "
> +                 lower and upper bounds at %L",
>                   &lvalue->where);
> 
> and possibly, instead of
> 
> -              gfc_error ("Either all or none of the upper bounds"
> -                 " must be specified at %L", &lvalue->where);
> +              gfc_error ("Rank remapping requires a "
> +                 BOUNDS_SPEC_LIST " at %L",
> +                 &lvalue->where);
>                return false;
> 
> use
> 
> " Rank remapping requires that all lower and upper bounds be specified"
> 
> ?
> 
> (And I am fairly certain that my versions are not the best possible
> ones...)
> 
> So, either something like what you propsed (but without the #defines)
> or something like what I wrote above would be OK.
> 
> Regards
> 
>     Thomas

[-- Attachment #2: patch-pr60091-v3 --]
[-- Type: text/plain, Size: 3676 bytes --]

Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 269715)
+++ gcc/fortran/expr.c	(working copy)
@@ -3703,6 +3703,7 @@
   gfc_ref *ref;
   bool is_pure, is_implicit_pure, rank_remap;
   int proc_pointer;
+  bool same_rank;
 
   lhs_attr = gfc_expr_attr (lvalue);
   if (lvalue->ts.type == BT_UNKNOWN && !lhs_attr.proc_pointer)
@@ -3724,6 +3725,7 @@
   proc_pointer = lvalue->symtree->n.sym->attr.proc_pointer;
 
   rank_remap = false;
+  same_rank = lvalue->rank == rvalue->rank;
   for (ref = lvalue->ref; ref; ref = ref->next)
     {
       if (ref->type == REF_COMPONENT)
@@ -3748,36 +3750,68 @@
 			       lvalue->symtree->n.sym->name, &lvalue->where))
 	    return false;
 
-	  /* When bounds are given, all lbounds are necessary and either all
-	     or none of the upper bounds; no strides are allowed.  If the
-	     upper bounds are present, we may do rank remapping.  */
+	  /* Fortran standard (e.g. F2018, 10.2.2 Pointer assignment):
+	   *
+	   * (C1017) If bounds-spec-list is specified, the number of
+	   * bounds-specs shall equal the rank of data-pointer-object.
+	   *
+	   * If bounds-spec-list appears, it specifies the lower bounds.
+	   *
+	   * (C1018) If bounds-remapping-list is specified, the number of
+	   * bounds-remappings shall equal the rank of data-pointer-object.
+	   *
+	   * If bounds-remapping-list appears, it specifies the upper and
+	   * lower bounds of each dimension of the pointer; the pointer target
+	   * shall be simply contiguous or of rank one.
+	   *
+	   * (C1019) If bounds-remapping-list is not specified, the ranks of
+	   * data-pointer-object and data-target shall be the same.
+	   *
+	   * Thus when bounds are given, all lbounds are necessary and either
+	   * all or none of the upper bounds; no strides are allowed.  If the
+	   * upper bounds are present, we may do rank remapping.  */
 	  for (dim = 0; dim < ref->u.ar.dimen; ++dim)
 	    {
-	      if (!ref->u.ar.start[dim]
-		  || ref->u.ar.dimen_type[dim] != DIMEN_RANGE)
+	      if (ref->u.ar.stride[dim])
 		{
-		  gfc_error ("Lower bound has to be present at %L",
+		  gfc_error ("Stride must not be present at %L",
 			     &lvalue->where);
 		  return false;
 		}
-	      if (ref->u.ar.stride[dim])
+	      if (!same_rank && (!ref->u.ar.start[dim] ||!ref->u.ar.end[dim]))
 		{
-		  gfc_error ("Stride must not be present at %L",
-			     &lvalue->where);
+		  gfc_error ("Rank remapping requires a "
+			     "list of %<lower-bound : upper-bound%> "
+			     "specifications at %L", &lvalue->where);
 		  return false;
 		}
+	      if (!ref->u.ar.start[dim]
+		  || ref->u.ar.dimen_type[dim] != DIMEN_RANGE)
+		{
+		  gfc_error ("Expected list of %<lower-bound :%> or "
+			     "list of %<lower-bound : upper-bound%> "
+			     "specifications at %L", &lvalue->where);
+		  return false;
+		}
 
 	      if (dim == 0)
 		rank_remap = (ref->u.ar.end[dim] != NULL);
 	      else
 		{
-		  if ((rank_remap && !ref->u.ar.end[dim])
-		      || (!rank_remap && ref->u.ar.end[dim]))
+		  if ((rank_remap && !ref->u.ar.end[dim]))
 		    {
-		      gfc_error ("Either all or none of the upper bounds"
-				 " must be specified at %L", &lvalue->where);
+		      gfc_error ("Rank remapping requires a "
+				 "list of %<lower-bound : upper-bound%> "
+				 "specifications at %L", &lvalue->where);
 		      return false;
 		    }
+		  if (!rank_remap && ref->u.ar.end[dim])
+		    {
+		      gfc_error ("Expected list of %<lower-bound :%> or "
+				 "list of %<lower-bound : upper-bound%> "
+				 "specifications at %L", &lvalue->where);
+		      return false;
+		    }
 		}
 	    }
 	}

[-- Attachment #3: patch-pr60091-testcases-v3 --]
[-- Type: text/plain, Size: 1995 bytes --]

Index: gcc/testsuite/gfortran.dg/pointer_remapping_3.f08
===================================================================
--- gcc/testsuite/gfortran.dg/pointer_remapping_3.f08	(revision 269715)
+++ gcc/testsuite/gfortran.dg/pointer_remapping_3.f08	(working copy)
@@ -3,6 +3,7 @@
 
 ! PR fortran/29785
 ! PR fortran/45016
+! PR fortran/60091
 ! Check for pointer remapping compile-time errors.
 
 ! Contributed by Daniel Kraft, d@domob.eu.
@@ -13,13 +14,13 @@
   INTEGER, POINTER :: vec(:), mat(:, :)
 
   ! Existence of reference elements.
-  vec(:) => arr ! { dg-error "Lower bound has to be present" }
-  vec(5:7:1) => arr ! { dg-error "Stride must not be present" }
-  mat(1:, 2:5) => arr ! { dg-error "Either all or none of the upper bounds" }
-  mat(2, 6) => arr ! { dg-error "Expected bounds specification" }
+  vec(:) => arr ! { dg-error "or list of 'lower-bound : upper-bound'" }
+  vec(5:7:1)  => arr ! { dg-error "Stride must not be present" }
+  mat(1:,2:5) => arr ! { dg-error "Rank remapping requires a list of " }
+  mat(1:3,4:) => arr ! { dg-error "Rank remapping requires a list of " }
+  mat(2, 6)   => arr ! { dg-error "Expected bounds specification" }
 
-  ! This is bound remapping not rank remapping!
-  mat(1:, 3:) => arr ! { dg-error "Different ranks" }
+  mat(1:,3:)  => arr ! { dg-error "Rank remapping requires a list of " }
 
   ! Invalid remapping target; for non-rank one we already check the F2008
   ! error elsewhere.  Here, test that not-contiguous target is disallowed
Index: gcc/testsuite/gfortran.dg/pointer_remapping_7.f90
===================================================================
--- gcc/testsuite/gfortran.dg/pointer_remapping_7.f90	(revision 269715)
+++ gcc/testsuite/gfortran.dg/pointer_remapping_7.f90	(working copy)
@@ -4,5 +4,5 @@
 !
   integer, target :: A(100)
   integer,pointer :: P(:,:)
-  p(10,1:) => A  ! { dg-error "Lower bound has to be present" }
+  p(10,1:) => A  ! { dg-error "or list of 'lower-bound : upper-bound'" }
   end

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR fortran/60091, patch] - Misleading error messages in rank-2 pointer assignment to rank-1 target
  2019-03-13 13:01     ` Aw: " Harald Anlauf
@ 2019-03-14 11:14       ` Dominique d'Humières
  0 siblings, 0 replies; 7+ messages in thread
From: Dominique d'Humières @ 2019-03-14 11:14 UTC (permalink / raw)
  To: Harald Anlauf; +Cc: Thomas Koenig, gfortran, gcc-patches



> Le 13 mars 2019 à 13:39, Harald Anlauf <anlauf@gmx.de> a écrit :
> 
> Hi Thomas,
> 
> I am not so convinced that "plain english" messages are the way to go,
> even if they appear better readable at first sight, if conciseness is
> lost.

Well, "Syntax error" is concise, but not really helpful!

>  The main reason is that there three variants of the messages,
> depending on context.  One of them refers to expecting either a
> bounds-specification-list or a bounds-remapping-list.
> 
> Do you prefer sth. like
> 
> "All lower bounds and all or none of the upper bounds must be specified"

This is what I expect for p(:,:)=>a and I won’t complain if "use the later for bound remapping" is added
when the target is a rank 1 array.

For mat(2, 6)   => arr I would prefer the above error rather than "Expected bounds specification ».

For p(1:,1:)=>a where a is a rank 1 target, I’ll go with

"all the upper bounds must be specified for bound remapping"

> or
> 
> "Either all or none of the upper bounds must be specified at (1)"

This is what I expect for p(1:3,1:)=>a with the same remark as above about bound remapping.

> (which we currently print in another context where it is wrong),
> 
> while other compilers print:
> 
> E.g. Crayftn:
> 
>  p(1 ,2:3) => t
>    ^            
> ftn-1768 crayftn: ERROR SUB, File = ptr-remap.f90, Line = 3, Column = 5 
>  Invalid bounds-spec-list or bounds-remapping-list for this pointer assignment.
> 
> E.g. Intel:
> 
> ptr-remap.f90(3): error #8524: The syntax of this data pointer assignment is incorrect: either 'bound spec' or 'bound remapping' is expected in this context.   [1]
>  p(1 ,2:3) => t
> ----^
> 
> Pointer remapping belongs IMHO to the 'more advanced’ features

Agreed

> and requires
> some technical insight to get it right, which is why I think the related
> error messages should be more technical and concise.

Here I disagree, the error message should try to tell the user what is wrong
without requiring any access to the standard.

> 
> I'll think for another day or two.
> 
> Thanks,
> Harald

These defines should probably be swapped

+#define BOUNDS_SPEC_LIST "list of %<lower-bound-expr:upper-bound-expr%>"
+#define BOUNDS_REMAPPING_LIST "list of %<lower-bound-expr:%>"

to match

> R1035 bounds-spec is lower-bound-expr :
> R1036 bounds-remapping is lower-bound-expr : upper-bound-exp

Dominique

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR fortran/60091, patch] - Misleading error messages in rank-2 pointer assignment to rank-1 target
  2019-03-12 22:49   ` Thomas Koenig
@ 2019-03-13  0:39     ` Steve Kargl
  2019-03-13 13:01     ` Aw: " Harald Anlauf
  2019-03-15 22:33     ` Harald Anlauf
  2 siblings, 0 replies; 7+ messages in thread
From: Steve Kargl @ 2019-03-13  0:39 UTC (permalink / raw)
  To: Thomas Koenig
  Cc: Harald Anlauf, Dominique d'Humières, gfortran, gcc-patches

On Tue, Mar 12, 2019 at 11:19:07PM +0100, Thomas Koenig wrote:
> Hi Harald,
> 
> > how about the attached version?  It is quite verbose and produces
> > messages like
> > 
> > Error: Expected list of 'lower-bound-expr:' or list of
> > 'lower-bound-expr:upper-bound-expr' at (1)
> 
> I think this way of specifying error messages
> 
> +#define BOUNDS_SPEC_LIST "list of %<lower-bound-expr:upper-bound-expr%>"
> 
> ...
> 
> +		  gfc_error ("Rank remapping requires a "
> +			     BOUNDS_SPEC_LIST " at %L",
>   			     &lvalue->where);
> 
> will cause trouble in translation of the error messages.
> 

I agree with Thomas here.  We should try to make the
translation of error message as painless as possible.

-- 
Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR fortran/60091, patch] - Misleading error messages in rank-2 pointer assignment to rank-1 target
  2019-03-11 20:16 ` Harald Anlauf
@ 2019-03-12 22:49   ` Thomas Koenig
  2019-03-13  0:39     ` Steve Kargl
                       ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Thomas Koenig @ 2019-03-12 22:49 UTC (permalink / raw)
  To: Harald Anlauf, Dominique d'Humières; +Cc: gfortran, gcc-patches

Hi Harald,

> how about the attached version?  It is quite verbose and produces
> messages like
> 
> Error: Expected list of 'lower-bound-expr:' or list of
> 'lower-bound-expr:upper-bound-expr' at (1)

I think this way of specifying error messages

+#define BOUNDS_SPEC_LIST "list of %<lower-bound-expr:upper-bound-expr%>"

...

+		  gfc_error ("Rank remapping requires a "
+			     BOUNDS_SPEC_LIST " at %L",
  			     &lvalue->where);

will cause trouble in translation of the error messages.

Could you maybe use something like

+		  gfc_error ("Rank remapping requires "
+			     lower and upper bounds at %L",
  			     &lvalue->where);

and possibly, instead of

-		      gfc_error ("Either all or none of the upper bounds"
-				 " must be specified at %L", &lvalue->where);
+		      gfc_error ("Rank remapping requires a "
+				 BOUNDS_SPEC_LIST " at %L",
+				 &lvalue->where);
  		      return false;

use

" Rank remapping requires that all lower and upper bounds be specified"

?

(And I am fairly certain that my versions are not the best possible
ones...)

So, either something like what you propsed (but without the #defines)
or something like what I wrote above would be OK.

Regards

	Thomas

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR fortran/60091, patch] - Misleading error messages in rank-2 pointer assignment to rank-1 target
  2019-03-11  9:37 Dominique d'Humières
@ 2019-03-11 20:16 ` Harald Anlauf
  2019-03-12 22:49   ` Thomas Koenig
  0 siblings, 1 reply; 7+ messages in thread
From: Harald Anlauf @ 2019-03-11 20:16 UTC (permalink / raw)
  To: Dominique d'Humières; +Cc: gfortran, gcc-patches

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

Hi Dominique,

how about the attached version?  It is quite verbose and produces
messages like

Error: Expected list of 'lower-bound-expr:' or list of
'lower-bound-expr:upper-bound-expr' at (1)

(I did check other compilers.  E.g. Intel and Oracle do print messages
using the 'legalese'.  But user-friendliness does count, too.)

OK for trunk?  Further comments?

Thanks,
Harald

On 03/11/19 10:22, Dominique d'Humières wrote:
> Hi Harald,
> 
> The patch looks good to me (although I did not test it), however I don’t like the standard legalese in the error messages.
> 
> IMO
> 
> R1035 bounds-spec is lower-bound-expr :
> R1036 bounds-remapping is lower-bound-expr : upper-bound-exp
> 
> should be rephrased in plain English.
> 
> Thanks for the work.
> 
> Dominique



[-- Attachment #2: patch-pr60091-v2 --]
[-- Type: text/plain, Size: 3639 bytes --]

Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 269593)
+++ gcc/fortran/expr.c	(working copy)
@@ -3703,6 +3703,7 @@
   gfc_ref *ref;
   bool is_pure, is_implicit_pure, rank_remap;
   int proc_pointer;
+  bool same_rank;
 
   lhs_attr = gfc_expr_attr (lvalue);
   if (lvalue->ts.type == BT_UNKNOWN && !lhs_attr.proc_pointer)
@@ -3724,6 +3725,7 @@
   proc_pointer = lvalue->symtree->n.sym->attr.proc_pointer;
 
   rank_remap = false;
+  same_rank = lvalue->rank == rvalue->rank;
   for (ref = lvalue->ref; ref; ref = ref->next)
     {
       if (ref->type == REF_COMPONENT)
@@ -3748,36 +3750,72 @@
 			       lvalue->symtree->n.sym->name, &lvalue->where))
 	    return false;
 
-	  /* When bounds are given, all lbounds are necessary and either all
-	     or none of the upper bounds; no strides are allowed.  If the
-	     upper bounds are present, we may do rank remapping.  */
+	  /* Fortran standard (e.g. F2018, 10.2.2 Pointer assignment):
+	   *
+	   * (C1017) If bounds-spec-list is specified, the number of
+	   * bounds-specs shall equal the rank of data-pointer-object.
+	   *
+	   * If bounds-spec-list appears, it specifies the lower bounds.
+	   *
+	   * (C1018) If bounds-remapping-list is specified, the number of
+	   * bounds-remappings shall equal the rank of data-pointer-object.
+	   *
+	   * If bounds-remapping-list appears, it specifies the upper and
+	   * lower bounds of each dimension of the pointer; the pointer target
+	   * shall be simply contiguous or of rank one.
+	   *
+	   * (C1019) If bounds-remapping-list is not specified, the ranks of
+	   * data-pointer-object and data-target shall be the same.
+	   *
+	   * Thus when bounds are given, all lbounds are necessary and either
+	   * all or none of the upper bounds; no strides are allowed.  If the
+	   * upper bounds are present, we may do rank remapping.  */
+
+#define BOUNDS_SPEC_LIST "list of %<lower-bound-expr:upper-bound-expr%>"
+#define BOUNDS_REMAPPING_LIST "list of %<lower-bound-expr:%>"
+
 	  for (dim = 0; dim < ref->u.ar.dimen; ++dim)
 	    {
-	      if (!ref->u.ar.start[dim]
-		  || ref->u.ar.dimen_type[dim] != DIMEN_RANGE)
+	      if (ref->u.ar.stride[dim])
 		{
-		  gfc_error ("Lower bound has to be present at %L",
+		  gfc_error ("Stride must not be present at %L",
 			     &lvalue->where);
 		  return false;
 		}
-	      if (ref->u.ar.stride[dim])
+	      if (!same_rank && (!ref->u.ar.start[dim] ||!ref->u.ar.end[dim]))
 		{
-		  gfc_error ("Stride must not be present at %L",
+		  gfc_error ("Rank remapping requires a "
+			     BOUNDS_SPEC_LIST " at %L",
 			     &lvalue->where);
 		  return false;
 		}
+	      if (!ref->u.ar.start[dim]
+		  || ref->u.ar.dimen_type[dim] != DIMEN_RANGE)
+		{
+		  gfc_error ("Expected " BOUNDS_REMAPPING_LIST " or "
+			     BOUNDS_SPEC_LIST " at %L",
+			     &lvalue->where);
+		  return false;
+		}
 
 	      if (dim == 0)
 		rank_remap = (ref->u.ar.end[dim] != NULL);
 	      else
 		{
-		  if ((rank_remap && !ref->u.ar.end[dim])
-		      || (!rank_remap && ref->u.ar.end[dim]))
+		  if ((rank_remap && !ref->u.ar.end[dim]))
 		    {
-		      gfc_error ("Either all or none of the upper bounds"
-				 " must be specified at %L", &lvalue->where);
+		      gfc_error ("Rank remapping requires a "
+				 BOUNDS_SPEC_LIST " at %L",
+				 &lvalue->where);
 		      return false;
 		    }
+		  if (!rank_remap && ref->u.ar.end[dim])
+		    {
+		      gfc_error ("Expected " BOUNDS_REMAPPING_LIST " or "
+				 BOUNDS_SPEC_LIST " at %L",
+				 &lvalue->where);
+		      return false;
+		    }
 		}
 	    }
 	}

[-- Attachment #3: patch-pr60091-testcases-v2 --]
[-- Type: text/plain, Size: 2011 bytes --]

Index: gcc/testsuite/gfortran.dg/pointer_remapping_3.f08
===================================================================
--- gcc/testsuite/gfortran.dg/pointer_remapping_3.f08	(revision 269593)
+++ gcc/testsuite/gfortran.dg/pointer_remapping_3.f08	(working copy)
@@ -3,6 +3,7 @@
 
 ! PR fortran/29785
 ! PR fortran/45016
+! PR fortran/60091
 ! Check for pointer remapping compile-time errors.
 
 ! Contributed by Daniel Kraft, d@domob.eu.
@@ -13,13 +14,13 @@
   INTEGER, POINTER :: vec(:), mat(:, :)
 
   ! Existence of reference elements.
-  vec(:) => arr ! { dg-error "Lower bound has to be present" }
-  vec(5:7:1) => arr ! { dg-error "Stride must not be present" }
-  mat(1:, 2:5) => arr ! { dg-error "Either all or none of the upper bounds" }
-  mat(2, 6) => arr ! { dg-error "Expected bounds specification" }
+  vec(:) => arr ! { dg-error "or list of 'lower-bound-expr:upper-bound-expr'" }
+  vec(5:7:1)  => arr ! { dg-error "Stride must not be present" }
+  mat(1:,2:5) => arr ! { dg-error "Rank remapping requires a list of " }
+  mat(1:3,4:) => arr ! { dg-error "Rank remapping requires a list of " }
+  mat(2, 6)   => arr ! { dg-error "Expected bounds specification" }
 
-  ! This is bound remapping not rank remapping!
-  mat(1:, 3:) => arr ! { dg-error "Different ranks" }
+  mat(1:,3:)  => arr ! { dg-error "Rank remapping requires a list of " }
 
   ! Invalid remapping target; for non-rank one we already check the F2008
   ! error elsewhere.  Here, test that not-contiguous target is disallowed
Index: gcc/testsuite/gfortran.dg/pointer_remapping_7.f90
===================================================================
--- gcc/testsuite/gfortran.dg/pointer_remapping_7.f90	(revision 269593)
+++ gcc/testsuite/gfortran.dg/pointer_remapping_7.f90	(working copy)
@@ -4,5 +4,5 @@
 !
   integer, target :: A(100)
   integer,pointer :: P(:,:)
-  p(10,1:) => A  ! { dg-error "Lower bound has to be present" }
+  p(10,1:) => A  ! { dg-error "or list of 'lower-bound-expr:upper-bound-expr'" }
   end

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PR fortran/60091, patch] - Misleading error messages in rank-2 pointer assignment to rank-1 target
@ 2019-03-11  9:37 Dominique d'Humières
  2019-03-11 20:16 ` Harald Anlauf
  0 siblings, 1 reply; 7+ messages in thread
From: Dominique d'Humières @ 2019-03-11  9:37 UTC (permalink / raw)
  To: anlauf; +Cc: gfortran, gcc-patches

Hi Harald,

The patch looks good to me (although I did not test it), however I don’t like the standard legalese in the error messages.

IMO

R1035 bounds-spec is lower-bound-expr :
R1036 bounds-remapping is lower-bound-expr : upper-bound-exp

should be rephrased in plain English.

Thanks for the work.

Dominique

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-03-15 22:24 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07 23:13 [PR fortran/60091, patch] - Misleading error messages in rank-2 pointer assignment to rank-1 target Harald Anlauf
2019-03-11  9:37 Dominique d'Humières
2019-03-11 20:16 ` Harald Anlauf
2019-03-12 22:49   ` Thomas Koenig
2019-03-13  0:39     ` Steve Kargl
2019-03-13 13:01     ` Aw: " Harald Anlauf
2019-03-14 11:14       ` Dominique d'Humières
2019-03-15 22:33     ` Harald Anlauf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).