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

[Janus Weil]

Hi Thomas,

> the attached patch warns about the dubious pointer assignments (see
> test case for details).

thanks for the patch! Sounds like a useful diagnostic.


> 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, 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?


> - 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?


> 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).


On top, some direct comments to your patch:

+  /* Warn for suspicious assignments like
+
+     pointer, real, dimension(:), contiguous :: p
+     real, dimension(10,10) :: a
+     p => a(:,:,2) or p => a(2:4,:)  */

I'm all for good and extensive comments, but instead of writing out
example code here, it might be more concise to just say something
like: "Check for assignments of contiguous pointers to non-contiguous
targets."

+      gfc_array_ref *ar;
+      int i;
+      bool do_warn;
+
+      do_warn = false;
+      ar = NULL;

Maybe add the initialization directly to the declaration here?

gfc_array_ref *ar = NULL;
bool do_warn = false;

The following could be replaced by "gfc_find_array_ref", I guess?

+      for (ref = rvalue->ref; ref; ref = ref->next)
+    {
+      if (ref->type == REF_ARRAY)
+        {
+          ar = &ref->u.ar;
+          break;
+        }
+    }

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.)


Cheers,
Janus