This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch, fortran] Warn about suspicious assignment to contiguous pointers


Hi Janus,



I think an unconditional warning is OK
in this case because

- Assigning to a pointer from an obvious non-contiguous target
   is not useful at all, that I can see

I guess you're talking about a *contiguous* pointer here,

Correct.

and in that
case I would argue that, beyond being "not useful", it's even illegal,
so why not throw a hard error, if we can infer at compile-time that
the target is non-contiguous?

Problem is, we cannot infer this from the tests done.
We would also have to add a test if the array is empty
or that it contains only a single element, and that (I think)
is a) impossible in the general case, and b) not worth the bother.


- Some language laywer will come up with the fact that it is,
   in fact, legal if the target is empty or has a single
   element only, so a hard error would be a rejects-valid.

Should be possible to detect and ignore such cases, shouldn't it?

Not in general.


However, I can also make this into a warning depending on
-Wall, if this is preferred.

As explained above, I'd vote for an error (or at least a conditional
warning, so that it can be disabled, like most(?) other gfortran
warnings).

Attached is a version which makes this a warning enabled by -Wall;
this should be enough to give people a heads-up.

I have introduced most of your comments into the revised patch.

Also I noticed that there is a function called
"gfc_is_simply_contiguous" in expr.c. Could that be useful for your
purpose? (Some of the code in there looks very similar to the logic
you're adding.)

There is a subtle distinction between "simply contiguous" where
the compiler _has_ to know at compile-time that the expression
is contigous (and failure to diagnose is is a compiler error)
and "contiguous" where the burden is on the programmer.
My patch is intended to be an aid to the programmer for
the "continuous" case.

Because of the "simple contiguous" thing, the function does
not quite match the requirements.

So, here is the updated patch.  Regression-tested on
powerpc-linux, make dvi and make pdf also passed.
OK for trunk?

Regards

	Thomas

2017-08-27  Thomas Koenig  <tkoenig@gcc.gnu.org>

        PR fortran/49232
        * expr.c (gfc_check_pointer_assign): Warn for
        suspicious assignments with contiguous pointrs if
        -Wcontiguous is given.
        * lang.opt (Wcontiguous): New option, implied
        by -Wall.
        * invoke.texi: Document -Wcontiguous.

2017-08-27  Thomas Koenig  <tkoenig@gcc.gnu.org>

        PR fortran/49232
        * gfortran.dg/contiguous_4.f90: New test.
Index: expr.c
===================================================================
--- expr.c	(Revision 251375)
+++ expr.c	(Arbeitskopie)
@@ -3802,6 +3802,54 @@
 	  }
     }
 
+  /* Warn for assignments of contiguous pointers to targets which might
+     not be contiguous.  */
+
+  if (warn_contiguous && lhs_attr.contiguous)
+    {
+      gfc_array_ref *ar;
+      int i;
+      bool do_warn = false;
+      
+      ar = gfc_find_array_ref (rvalue, true);
+      if (ar && ar->type == AR_SECTION)
+	{
+	  for (i = 0; i < ar->dimen; i++)
+	    {
+	      /* Check for p => a(:,:,2).  */
+	      if (ar->dimen_type[i] == DIMEN_RANGE && ar->stride[i]
+		  && (ar->stride[i]->expr_type != EXPR_CONSTANT
+		      || (ar->stride[i]->expr_type == EXPR_CONSTANT
+			  && mpz_cmp_si (ar->stride[i]->value.integer, 1))))
+		{
+		  do_warn = true;
+		  break;
+		}
+	    }
+	  if (!do_warn && ar->dimen > 1)
+	    {
+	      /* Check for p => a(2:4,:) */
+	      for (i = 0; i < ar->dimen - 1; i++)
+		{
+		  if ((ar->start[i] && ar->as->lower[i]
+		       && gfc_dep_compare_expr (ar->start[i], ar->as->lower[i])
+		       != 0)
+		      || (ar->end[i] && ar->as->upper[i]
+			  && gfc_dep_compare_expr (ar->end[i], ar->as->upper[i])
+			  != 0))
+		    {
+		      do_warn = true;
+		      break;
+		    }
+		}
+	    }
+	}
+      if (do_warn)
+	gfc_warning (OPT_Wcontiguous, "Assignment to contiguous pointer "
+		     "from possibly non-contiguous target at %L",
+		     &rvalue->where);
+    }
+
   /* Warn if it is the LHS pointer may lives longer than the RHS target.  */
   if (warn_target_lifetime
       && rvalue->expr_type == EXPR_VARIABLE
Index: invoke.texi
===================================================================
--- invoke.texi	(Revision 251375)
+++ invoke.texi	(Arbeitskopie)
@@ -145,7 +145,7 @@
 @xref{Error and Warning Options,,Options to request or suppress errors
 and warnings}.
 @gccoptlist{-Waliasing -Wall -Wampersand -Wargument-mismatch -Warray-bounds
--Wc-binding-type -Wcharacter-truncation @gol
+-Wc-binding-type -Wcharacter-truncation -Wcontiguous @gol
 -Wconversion -Wfunction-elimination -Wimplicit-interface @gol
 -Wimplicit-procedure -Wintrinsic-shadow -Wuse-without-only -Wintrinsics-std @gol
 -Wline-truncation -Wno-align-commons -Wno-tabs -Wreal-q-constant @gol
@@ -797,7 +797,7 @@
 This currently includes @option{-Waliasing}, @option{-Wampersand},
 @option{-Wconversion}, @option{-Wsurprising}, @option{-Wc-binding-type},
 @option{-Wintrinsics-std}, @option{-Wtabs}, @option{-Wintrinsic-shadow},
-@option{-Wline-truncation}, @option{-Wtarget-lifetime},
+@option{-Wline-truncation}, @option{-Wtarget-lifetime}, @option{-Wcontiguous},
 @option{-Winteger-division}, @option{-Wreal-q-constant}, @option{-Wunused}
 and @option{-Wundefined-do-loop}.
 
@@ -872,6 +872,13 @@
 @option{-Werror=line-truncation} such that truncations are reported as
 error.
 
+@item -Wcontiguous
+@opindex @code{Wcontiguous}
+@cindex warnings, contiguous
+Warn when a pointer assignment to a contiguous pointer is found where
+the target is probably not contiguous.  This option is implied by
+@option{-Wall}.
+
 @item -Wconversion
 @opindex @code{Wconversion}
 @cindex warnings, conversion
Index: lang.opt
===================================================================
--- lang.opt	(Revision 251375)
+++ lang.opt	(Arbeitskopie)
@@ -229,6 +229,10 @@
 Fortran Warning Var(warn_compare_reals) LangEnabledBy(Fortran,Wextra)
 Warn about equality comparisons involving REAL or COMPLEX expressions.
 
+Wcontiguous
+Fortran Warning Var(warn_contiguous) LangEnabledBy(Fortran,Wall)
+Warn about suspect assignments to contiguous pointers
+
 Wconversion
 Fortran Var(warn_conversion) Warning LangEnabledBy(Fortran,Wall)
 ; Documented in C
! { dg-do compile }
! { dg-additional-options "-Wcontiguous" }
program cont_01_neg
  implicit none
  real, pointer, contiguous :: r(:)
  real, pointer, contiguous :: r2(:,:)
  real, target :: x(45)
  real, target :: x2(5,9)
  integer :: i

  x = (/ (real(i),i=1,45) /)
  x2 = reshape(x,shape(x2))
  r => x(::3)   ! { dg-warning "Assignment to contiguous pointer" }
  r2 => x2(2:,:) ! { dg-warning "Assignment to contiguous pointer" }
  r2 => x2(:,2:3)

end program

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]