From patchwork Wed Aug 6 19:23:02 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andre Vehreschild X-Patchwork-Id: 377389 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id A345414009C for ; Thu, 7 Aug 2014 05:23:29 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type; q=dns; s=default; b=RW+qIEvTf66QMAyA SZ0m4mGm+2IfVahJdZM9bq0so5XVUchBLts975V4zOuXJYX5Nr/gIo7y99aBYH2+ JRsDAtlWtfMyY4hMEp8KzS8N2wrZey/RNHQBrPNe78pKgUdlkCvqz0/R0L7swceH 8IaBa+jy39siqqv27hLLNTuLIy8= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type; s=default; bh=gxsqXLchOAYZywPgE1ArO9 kjrnc=; b=XD4aNnKhDIchWpjZZxtZ0RLjZD5zmDhZjxyrtbuDthJUcQOdS66YY+ uhctXoh6Izsa7tIhIexi7mxoab+BWDqNxRB47ItYqElLWImlb5qNr58cOoBZw+mB 7wzXuF62H2fomlA+xa9X5sJVk9mR2lFnLFutOy+qihkoJ2x2exy7w= Received: (qmail 18166 invoked by alias); 6 Aug 2014 19:23:17 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 18148 invoked by uid 89); 6 Aug 2014 19:23:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mout.gmx.net Received: from mout.gmx.net (HELO mout.gmx.net) (212.227.15.18) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 06 Aug 2014 19:23:14 +0000 Received: from vepi2.private ([84.63.36.188]) by mail.gmx.com (mrgmx002) with ESMTPSA (Nemesis) id 0MGSDw-1XAlDd2JWq-00DIIw; Wed, 06 Aug 2014 21:23:05 +0200 Date: Wed, 6 Aug 2014 21:23:02 +0200 From: Andre Vehreschild To: fortran@gcc.gnu.org Cc: Mikael Morin , fxcoudert@gmail.com, gcc-patches@gcc.gnu.org, Dominique =?ISO-8859-1?B?ZCdIdW1p6HJlcw==?= Subject: [PATCH, Fortran] PR fortran/60414 fix ICE was: PR 60414: Patch proposal Message-ID: <20140806212302.4bf07e11@vepi2.private> In-Reply-To: <53D3FF8A.106@sfr.fr> References: <20140721072605.0D680105@mailhost.lps.ens.fr> <20140721150350.10b35dd3@vepi2.private> <911AF20A-F5E2-474F-858A-BF5CE56235D5@lps.ens.fr> <53D3FF8A.106@sfr.fr> MIME-Version: 1.0 X-UI-Out-Filterresults: notjunk:1; Hi, thanks for all the input. The issue to patch is an ICE while compiling a call of a generic method using an array reference, e.g., this.Check(vec(1)) where Check aggregates an implementation for an array parameter and one with a scalar parameter. Based on Mikael's input, I analyzed the code further. The original lines to consider are: gcc/fortran/interface.c: 2155++ /* If the rank is the same or the formal argument has assumed-rank. */ if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1) return 1; if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as && CLASS_DATA (actual)->as->rank == symbol_rank (formal)) return 1; While the first conditional is checking the rank of the formal argument against the rank *evaluated* for the actual argument, is the second one is checking the actual argument's "un-refed" rank to the formal ones'. This results in the later compile steps in an ICE, when the un-generic-ed function with the array argument is used for the scalar argument. I can't see what the purpose of the second conditional is (besides producing the error now). The code is rather old and I think that with introduction of generics, where the first conditional was introduced, the second one should have been removed. This is what the attached patch is for. The patch also adds a testcase for this ICE. *** gcc/fortran/Changelog *** 2014-08-06 Andre Vehreschild PR fortran/60414 * interface.c (compare_parameter): Fixing ICE when argument of a generic is a reference into an array. *** gcc/fortran/Changelog *** *** gcc/testsuite/Changelog *** 2014-08-06 Andre Vehreschild * gfortran.dg/unlimited_polymorphism_18.f90: Check according to PR fortran/60414 *** gcc/testsuite/Changelog *** Bootstrapped and regtested on x86_64-unkown-linux-gnu. Regards, Andre On Sat, 26 Jul 2014 21:20:42 +0200 Mikael Morin wrote: > Hello, thanks for your contribution. > > here are some comments about the patch: > > Le 21/07/2014 15:03, Andre Vehreschild a écrit : > > diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog > > index c33936b..cb01a13 100644 > > --- a/gcc/fortran/ChangeLog > > +++ b/gcc/fortran/ChangeLog > Changelogs are preferably provided outside of the patch as they almost > always conflict when applying it. > > > @@ -1,3 +1,11 @@ > > +2014-07-19 Andre Vehreschild > > + > > + PR fortran/60414 > > + * interface.c (compare_parameter): Fix compile bug: During > > resolution > > + of generic an array reference in the actual argument was not > > + respected. Fixed by checking, if the ref member is non-null. > You don't need to explain why (in the Changelog I mean), only _what_ is > changed should be there. > > > Testcase > > + unlimited_polymorphism_18.f90 add. > No need for that here; it's redundant with the testsuite Changelog. > > > + > > 2014-06-15 Tobias Burnus > > > > * symbol.c (check_conflict): Add codimension conflict with > > diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c > > index b210d18..d84c888 100644 > > --- a/gcc/fortran/interface.c > > +++ b/gcc/fortran/interface.c > > @@ -2156,7 +2156,11 @@ compare_parameter (gfc_symbol *formal, gfc_expr > > *actual, > > if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1) > > return 1; > > > > + /* Only check ranks compatibility, when the actual argument is not a > > + reference of an array (foo(i)). A reference into an array is assumed > > + when actual->ref is non null. */ > > if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as > > + && !actual->ref > > && CLASS_DATA (actual)->as->rank == symbol_rank (formal)) > > return 1; > > > I think you have spotted the right place where the bug happens, but the > patch provided is not completely right for the following reason: > > actual->ref points to the beginning of the reference chain, so > for example if actual is the gfc_expr struct for "var%comp%vec", > actual->ref points to the gfc_ref struct for "comp". > Furthermore, you have to be aware that even bare array variables > without sub-reference get an implicit full array reference in the AST, so > if actual is the gfc_expr struct for "array_var", actual->ref is > non-null and points (indirectly) to a gfc_array_ref of type AR_FULL. > > So if you want to check for the absence of trailing sub-reference, you > have to walk down to the last reference chain. > But then you have to also support the case "var%array_comp%vec(1)" which > is supposed to have the rank of array_comp. > In fact I think the conditional supposed to support the CLASS cases is > completely bogus, and I don't see why the generic conditional just above > it wouldn't also support CLASS cases just fine. > Did you try removing the CLASS conditional entirely? > > Mikael > diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c index b210d18..9eddcdf 100644 --- a/gcc/fortran/interface.c +++ b/gcc/fortran/interface.c @@ -2156,10 +2156,6 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual, if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1) return 1; - if (actual->ts.type == BT_CLASS && CLASS_DATA (actual)->as - && CLASS_DATA (actual)->as->rank == symbol_rank (formal)) - return 1; - rank_check = where != NULL && !is_elemental && formal->as && (formal->as->type == AS_ASSUMED_SHAPE || formal->as->type == AS_DEFERRED) diff --git a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_18.f90 b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_18.f90 new file mode 100644 index 0000000..9044199 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_18.f90 @@ -0,0 +1,69 @@ +! { dg-do compile } +! Testing fix for +! PR fortran/60414 +! +module m + implicit none + Type T + real, public :: expectedScalar; + contains + procedure :: FCheck + procedure :: FCheckArr + generic :: Check => FCheck, FCheckArr + end Type + +contains + + subroutine FCheck(this,X) + class(T) this + class(*) X + real :: r + select type (X) + type is (real) + if ( abs (X - this%expectedScalar) > 0.0001 ) then + call abort() + end if + class default + call abort () + end select + end subroutine FCheck + + subroutine FCheckArr(this,X) + class(T) this + class(*) X(:) + integer i + do i = 1,6 + this%expectedScalar = i - 1.0 + call this%FCheck(X(i)) + end do + end subroutine FCheckArr + + subroutine CheckTextVector(vec, n, scal) + integer, intent(in) :: n + class(*), intent(in) :: vec(n) + class(*), intent(in) :: scal + integer j + Type(T) :: Tester + + ! Check full vector + call Tester%Check(vec) + ! Check a scalar of the same class like the vector + Tester%expectedScalar = 5.0 + call Tester%Check(scal) + ! Check an element of the vector, which is a scalar + j=3 + Tester%expectedScalar = 2.0 + call Tester%Check(vec(j)) + + end subroutine CheckTextVector + +end module + +program test + use :: m + implicit none + + real :: vec(1:6) = (/ 0, 1, 2, 3, 4, 5 /) + call checktextvector(vec, 6, 5.0) +end program test +