diff mbox series

[fortran] Fix PR 83012, rejects-valid regression with contiguous pointer

Message ID b0ed27bf-07fd-c6d1-9ddc-1c6b94212191@netcologne.de
State New
Headers show
Series [fortran] Fix PR 83012, rejects-valid regression with contiguous pointer | expand

Commit Message

Thomas Koenig Nov. 17, 2017, 5:38 p.m. UTC
Hello world,

the attached patch fixes the PR by looking at the function interface if
one exists.

Regression-tested. OK for trunk?

Regards

	Thomas

2017-11-17  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/83012
         * expr.c (gfc_is_simply_contiguous): If a function call through a
         class variable is done through a reference, check the function's
         interface.

2017-11-17  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/83012
         * gfortran.dg/contiguous_5.f90: New test.

Comments

Paul Richard Thomas Nov. 17, 2017, 8:03 p.m. UTC | #1
Hi Thomas,

This is OK.

Thanks

Paul


On 17 November 2017 at 17:38, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Hello world,
>
> the attached patch fixes the PR by looking at the function interface if
> one exists.
>
> Regression-tested. OK for trunk?
>
> Regards
>
>         Thomas
>
> 2017-11-17  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
>         PR fortran/83012
>         * expr.c (gfc_is_simply_contiguous): If a function call through a
>         class variable is done through a reference, check the function's
>         interface.
>
> 2017-11-17  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
>         PR fortran/83012
>         * gfortran.dg/contiguous_5.f90: New test.
Steve Kargl Nov. 17, 2017, 9:23 p.m. UTC | #2
Thomas,

Even though Paul OK the patch, I have a question below.


On Fri, Nov 17, 2017 at 06:38:20PM +0100, Thomas Koenig wrote:
> +    {
> +      if (expr->value.function.esym)
> +	return expr->value.function.esym->result->attr.contiguous;
> +      else
> +	{
> +	  /* We have to jump through some hoops if this is a vtab entry.  */
> +	  gfc_symbol *s;
> +	  gfc_ref *r, *rc;
> +
> +	  s = expr->symtree->n.sym;
> +	  if (s->ts.type != BT_CLASS)
> +	    return false;
> +	  
> +	  rc = NULL;
> +	  for (r = expr->ref; r; r = r->next)
> +	    if (r->type == REF_COMPONENT)
> +	      rc = r;

Should you have a break here?  As I understand it, you're walking a 
list, so you could have r, r->next, r->next->next, and so on.   Is
it possible to have r->next->type = REF_COMPONENT and 
r->next->next->type = REF_COMPONENT, where you end up with the wrong
one?
Thomas Koenig Nov. 18, 2017, 3:48 p.m. UTC | #3
Steve,

>> +	  for (r = expr->ref; r; r = r->next)
>> +	    if (r->type == REF_COMPONENT)
>> +	      rc = r;
> 
> Should you have a break here?  As I understand it, you're walking a
> list, so you could have r, r->next, r->next->next, and so on.   Is
> it possible to have r->next->type = REF_COMPONENT and
> r->next->next->type = REF_COMPONENT, where you end up with the wrong
> one?

The point is to have the last of the r->next->next->next chain that
is a REF_COMPONENT (which I assign to rc, which I later use).

In the test case, it is indeed expr->ref->next that is of
interest, but there could be other references in between,
the type could be part of another type or there could be an
array reference - thus the loop, which should catch such
cases.

I tried to come up with a test case that breaks the patch, but I didn't
manage to do so.

Here's part of a debug session on the test case (breakpoint was in
gfc_is_simply_contiguous, the second time it was hit):

(gdb) p expr->ref->next.u.c.component
$13 = (gfc_component *) 0x23f1e90
(gdb) p expr->ref->next.u.c.component->ts.interface
$14 = (gfc_symbol *) 0x23ee510
(gdb) p expr->ref->next.u.c.component->ts.interface->attr.contiguous
$16 = 1

Regards

	Thomas
diff mbox series

Patch

Index: expr.c
===================================================================
--- expr.c	(Revision 254408)
+++ expr.c	(Arbeitskopie)
@@ -5185,8 +5185,31 @@  gfc_is_simply_contiguous (gfc_expr *expr, bool str
   gfc_symbol *sym;
 
   if (expr->expr_type == EXPR_FUNCTION)
-    return expr->value.function.esym
-	   ? expr->value.function.esym->result->attr.contiguous : false;
+    {
+      if (expr->value.function.esym)
+	return expr->value.function.esym->result->attr.contiguous;
+      else
+	{
+	  /* We have to jump through some hoops if this is a vtab entry.  */
+	  gfc_symbol *s;
+	  gfc_ref *r, *rc;
+
+	  s = expr->symtree->n.sym;
+	  if (s->ts.type != BT_CLASS)
+	    return false;
+	  
+	  rc = NULL;
+	  for (r = expr->ref; r; r = r->next)
+	    if (r->type == REF_COMPONENT)
+	      rc = r;
+
+	  if (rc == NULL || rc->u.c.component == NULL
+	      || rc->u.c.component->ts.interface == NULL)
+	    return false;
+
+	  return rc->u.c.component->ts.interface->attr.contiguous;
+	}
+    }
   else if (expr->expr_type != EXPR_VARIABLE)
     return false;