Message ID | 20140806212302.4bf07e11@vepi2.private |
---|---|
State | New |
Headers | show |
Hello, Le 06/08/2014 21:23, Andre Vehreschild a écrit : > Hi, > [...] > > *** gcc/fortran/Changelog *** > 2014-08-06 Andre Vehreschild <vehre@gmx.de> > > PR fortran/60414 > * interface.c (compare_parameter): Fixing ICE when argument > of a generic is a reference into an array. > *** gcc/fortran/Changelog *** The ChangeLog format is good, but the text is not very helpful/descriptive for someone hunting a bug in compare_parameter in the future. You can say (for example): Remove class argument rank check short circuit. > > *** gcc/testsuite/Changelog *** > 2014-08-06 Andre Vehreschild <vehre@gmx.de> > > * gfortran.dg/unlimited_polymorphism_18.f90: Check according to > PR fortran/60414 > *** gcc/testsuite/Changelog *** You should add PR fortran/60414 before like in the gcc/fortran Changelog, and then the text just need to say new/new file/new test (see what the other contributors use in the rest of the file). > > Bootstrapped and regtested on x86_64-unkown-linux-gnu. > The patch looks good to me. With the ChangeLog fixes above, OK if/when the copyright assignment is settled. Thanks Mikael
As Mikael said in https://gcc.gnu.org/ml/fortran/2014-08/msg00047.html > the testcase should check that the code generated is actually working, > not just that the ICE disappeared. ... thus I think the test should be run, i.e., '! { dg-do compile }' should be replaced with '! { dg-do run }' (I have checked that the test succeeds). Dominique
Le 17/08/2014 14:26, Dominique Dhumieres a écrit : > As Mikael said in https://gcc.gnu.org/ml/fortran/2014-08/msg00047.html > >> the testcase should check that the code generated is actually working, >> not just that the ICE disappeared. ... > Well, this is for another patch where deferred character variable are made acceptable as argument to unlimited polymorphic dummies. Here the ICE comes (if I remember correctly) from the wrong generic procedure being picked, so there is not really some new feature enabled with the patch. > thus I think the test should be run, i.e., '! { dg-do compile }' should > be replaced with '! { dg-do run }' (I have checked that the test succeeds). > I don't have a strong opinion for it, but I'm OK with that change. In fact the initial test was a run one, and it has been changed to compile. Andre: why? Mikael
Hi, On Sun, 17 Aug 2014 15:06:02 +0200 Mikael Morin <mikael.morin@sfr.fr> wrote: > Le 17/08/2014 14:26, Dominique Dhumieres a écrit : > > As Mikael said in https://gcc.gnu.org/ml/fortran/2014-08/msg00047.html > > > >> the testcase should check that the code generated is actually working, > >> not just that the ICE disappeared. ... > > > Well, this is for another patch where deferred character variable are > made acceptable as argument to unlimited polymorphic dummies. > Here the ICE comes (if I remember correctly) from the wrong generic > procedure being picked, so there is not really some new feature enabled > with the patch. This is correct so far. > > > thus I think the test should be run, i.e., '! { dg-do compile }' should > > be replaced with '! { dg-do run }' (I have checked that the test succeeds). > > > I don't have a strong opinion for it, but I'm OK with that change. > In fact the initial test was a run one, and it has been changed to > compile. Andre: why? I was asked to move to compile only, because a run test takes a lot of time. I was told that the run test compiles the code multiple times with different optimization. This issue was deemed to be solely on the compile stage and was not influenced by optimization. Therefore I agreed to switch to dg-do compile. That the test is fine for running, too, is merely for my training of how to do that. My opinion is, that dg-do compile is sufficient to prove, that PR60414 is resolved, because that is the sole purpose of the patch. I understand Dominique wanting to have the dg-do run, because the effectiveness of the patch is only shown on running the test. Is there a compromise of running a test, but only for one optimization stage? Then may be we can do that. - Andre
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 +