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
