Re: Constant and pass-by-reference

[Mark Eggleston <mark dot eggleston at codethink dot co dot uk>]

On 21/06/2019 14:58, Steve Kargl wrote:
> On Fri, Jun 21, 2019 at 09:29:53AM +0100, Mark Eggleston wrote:
> > On 18/06/2019 14:22, Arjen Markus wrote:
> > > Hello Hafiz,
> > >
> > > I do not know if the compiler can be expected to catch all such cases
> > > - after all, the module may be compiled and stored in some library and
> > > then used in a separate build step. In that case the information that
> > > the actual argument is not a variable would be lost. You can, however,
> > > help the compiler by stating the intent for the dummy argument (var):
> > >
> > > integer, intent(out) :: var
> > >
> > > Now, the compiler will complain when it tries to compile the main program.
> > I have also come across this problem. In my opinion it is better to
> > catch such a fault at compile time rather than run time.
> >
> > A scheme that checks a dummy variable's intent is OUT or INOUT when a
> > value is assigned and issues a warning would help.
> >
> > Such a warning could be controlled by -Wall with perhaps an new option
> > -Wintent. When used with -Werror the program will then fail to compile.
> >
> > This would encourage the use of intent with dummy.
> I suspect that this will have a high false positive rate.

I have a preliminary patch that implements this scheme (attached). 
Initially it was unaware of the VALUE attribute and was later changed. 
Running the gfortran test using "make -j 8 check-fortran" resulted in 8 
failures from 3 test cases (earlier version):

c_by_val_5.f90
coarray_24.f90
warn_concat.f90

Once the patch was aware of VALUE, the failures for c_by_val_5.f90 no 
longer occurred. Adding intent to the dummy variables in coarray_24.f90 
and warn_concart.f90 Rrduced to the failures to zero.

I not sure what you mean by "false positive" unless it is for FORTRAN 
code earlier than Fortran 90 which does not have intent. Dummy variables 
can simply have INTENT attributes added resulting in clearer code.

The patch currently includes this warning message:

"Dummy variable %qs in assignment has no INTENT specified, use 
INTENT(INOUT) or INTENT(OUT) at %L"

I'm not happy with it as it was written before taking into account the 
VALUE attribute. Any suggestions will be appreciated.

I do think that this patch will be valuable as it can help catch bugs at 
compile time instead of run time. If there is a consensus that the use 
of -Wall is a bad idea I can disassociate it from -Wintent.

> My Makefiles already contain
>
> # gfortran is too noisy
> FFLAGS += -Wno-maybe-uninitialized -Wno-conversion -Wno-integer-division
>
> to suppress issues with these options.
>
> I would not support adding it to -Wall.
--
https://www.codethink.co.uk/privacy.html

>From 325fe828fd45084027e6c9b75f3302fce6f720af Mon Sep 17 00:00:00 2001
From: Mark Eggleston <markeggleston@codethink.com>
Date: Mon, 24 Jun 2019 09:51:15 +0100
Subject: [PATCH] Constant and pass-by-reference

---
 gcc/fortran/expr.c                        | 7 +++++++
 gcc/fortran/lang.opt                      | 4 ++++
 gcc/testsuite/gfortran.dg/coarray_24.f90  | 2 +-
 gcc/testsuite/gfortran.dg/warn_concat.f90 | 2 +-
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index 7b391dd3827..6ea07557da2 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -6095,6 +6095,13 @@ gfc_check_vardef_context (gfc_expr* e, bool pointer, bool alloc_obj,
 	}
     }
 
+  static const char* assignment = "assignment";
+  if (warn_intent && sym->attr.dummy && sym->attr.intent == INTENT_UNKNOWN &&
+    !sym->attr.value && strncmp(context, assignment, strlen(assignment))==0)
+    gfc_warning (OPT_Wintent, "Dummy variable %qs in assignment has no INTENT "
+		 "specified, use INTENT(INOUT) or INTENT(OUT) at %L",
+		 sym->name, &e->where);
+
   if (check_intentin
       && (sym->attr.intent == INTENT_IN
 	  || (sym->attr.select_type_temporary && sym->assoc
diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
index 7d9fd3e048c..3cf468485bd 100644
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -265,6 +265,10 @@ Winteger-division
 Fortran Warning Var(warn_integer_division) LangEnabledBy(Fortran,Wall)
 Warn about constant integer divisions with truncated results.
 
+Wintent
+Fortran Warning Var(warn_intent)  LangEnabledBy(Fortran,Wall)
+Warn of assignment to dummy variables that have no intent specified.
+
 Wline-truncation
 Fortran Warning Var(warn_line_truncation) LangEnabledBy(Fortran,Wall) Init(-1)
 Warn about truncated source lines.
diff --git a/gcc/testsuite/gfortran.dg/coarray_24.f90 b/gcc/testsuite/gfortran.dg/coarray_24.f90
index d8d92816d26..66c256f3f1a 100644
--- a/gcc/testsuite/gfortran.dg/coarray_24.f90
+++ b/gcc/testsuite/gfortran.dg/coarray_24.f90
@@ -15,7 +15,7 @@ call doubtful_valid(myCaf)  ! { dg-warning "to allocatable, noncoarray dummy" }
 call invalid(myCaf)         ! { dg-error "to allocatable, noncoarray, INTENT.OUT. dummy" }
 contains
   subroutine doubtful_valid(x)
-    integer, allocatable :: x(:)
+    integer, allocatable, intent(inout) :: x(:)
     ! Valid as x's allocation status is not touched.
     x(1) = 7
   end subroutine doubtful_valid
diff --git a/gcc/testsuite/gfortran.dg/warn_concat.f90 b/gcc/testsuite/gfortran.dg/warn_concat.f90
index 8006dd392d2..308e2970102 100644
--- a/gcc/testsuite/gfortran.dg/warn_concat.f90
+++ b/gcc/testsuite/gfortran.dg/warn_concat.f90
@@ -3,7 +3,7 @@
 ! PR 79929 - this used to give a warning.
 ! Test case by Harald Anlauf.
 subroutine gfcbug138 (yerrmsg)
-  character(*) :: yerrmsg
+  character(*), intent(inout) :: yerrmsg
   yerrmsg = ""
   yerrmsg = "bug: " // yerrmsg
 end subroutine gfcbug138
-- 
2.11.0