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