Re: PING: [patch, fortran] PR 27997: Implement F2003-style array constructor with typespec

[Tobias Burnus <burnus at net-b dot de>]

Daniel Kraft wrote:
> BTW, is it generally ok to have as many testcases as possible, or is
> there some "upper limit" on the number you think reasonable for one
> special feature?  And BTW, is there a way to make dejagnu run single
> tests so I can test my dejagnu-testcase without having to do a full
> check-gfortran?
In general, a comprehensive test is good. For every bug and for every 
feature representive tests should be included and in tendency more is 
better than less. However, every test makes running the test suite 
longer. This not only affects gfortraners ("make check-gfortran and  
"make check" in libgomp), but also all middle-end developers which have 
to do "make check" and check all of C, C++, Fortran, Java, ADA, ...

Nevertheless, so far I have not seen any patch which has been rejected 
because it had too many test cases, but I have seen requests for more tests.

(In principle, gfortran misses test cases for correctly working 
old/basic features. As they work no one feels like adding tests for 
those, which makes it possible that gfortran regresses without anyone 
noticing.)


My personal guideline:

* Fewer files are faster: If you have 5 very short files of valid code, 
I would combine them into one. (If they get lengthier or more 
complicated, they can be in separate files)

* Usually, one or maximally two tests which fail with -std= < 
implemented standard (e.g. -std=f95 test for F2003 feature) is enough.

* For invalid tests which should be rejected for all -std=, you probably 
need to have more than one file as the error recovery often fails after 
a few invalid tests and thus one cannot add more invalid tests and hence 
needs to start a new file.

* Consider using "dg-do compile" which is faster than "dg-do run" (which 
is compiled and run with several options), but include also some "dg-do 
run" tests; sometimes a new feature does not work although it compiles; 
sometimes the failures are restricted to certain systems such as PowerPC 
or s390.

* Try to come up with wierd combinations of valid  (or of invalid) code: 
Such as ENTRY, nested constructors, implicit typing, Bind(C), zero-sized 
arrays, fixed-format source, etc. This increases the chance that you 
find a bug in either your patch or in other parts of gfortran.

(In general I would advice to cross test the tests with other compilers, 
but this is difficult with F2003 features as one might not have access 
to another compiler which supports a given feature.)


Regarding valid tests: Cross check in the program the results; sometimes 
adding a few extra IFs shows that assumptions might be wrong. For instance:

 character(len=6) :: a; a= ''; a ='123';
 if(a /= '123' .or. any(a(i:i) /= achar(32), i =4,6)) call abort()
 a= 'XXXXXX'; a ='123';
 if(a /= '123' .or. any(a(i:i) /= achar(32), i =4,6)) call abort()

i.e. check that the padding is done correctly. (For some cases, gfortran 
sometimes did not pad character variables with ' ' until relatively 
recently.)

And one should also try to check REAL(10)/REAL(16). They tend to get 
forgotten and have sometimes bugs of their own. For the syntax, see: 
http://gcc.gnu.org/ml/fortran/2008-04/msg00248.html

If you have valgrind, you should also run valgrind both on f951 when 
compiling a test case and on the resulting test case; this is quite 
effective in finding certain kinds of programming errors. (To find the 
command line, which is needed for valgrind, compile with the "-v" 
option, search for the ".../f951 " line and use this as argument to 
valgind. Running "valgrind gfortran" does not make sense as "gfortran" 
is only a wrapper which calls the actual compiler "f951".)

* * *

> > +      if (flag_bounds_check && !typespec_ctor)
> >
> > I've tried to trigger that code and could not see it work. I cannot 
> see a runtime error, neither with
>
> For this I didn't do anything by now, if it is ok I suggest we look at 
> this separatly?
I think it check is ok and it is/should only be triggered without 
typespec. I believe this works (there should be a test case somewhere in 
the test suite).


Tobias