Re: [PATCH 1/2] analyzer: gfortran testsuite support

[David Malcolm <dmalcolm@redhat.com>]

On Sun, 2020-02-09 at 12:55 -0800, Steve Kargl wrote:
> On Sun, Feb 09, 2020 at 09:15:46PM +0100, Toon Moene wrote:
> > On 2/6/20 9:01 PM, David Malcolm wrote:
> > 
> > > PR analyzer/93405 reports an ICE when attempting to use
> > > -fanalyzer on
> > > certain gfortran code.  The second patch in this kit fixes that,
> > > but
> > > in the meantime I need somewhere to put regression tests for
> > > -fanalyzer
> > > with gfortran.
> > > 
> > > This patch adds a gfortran.dg/analyzer subdirectory with an
> > > analyzer.exp,
> > > setting DEFAULT_FFLAGS on the tests run within it.
> > 
> > I have seen no objections against this proposal, so please go
> > ahead.
> > 
> 
> Perhaps, there are no objections because the people who contribute
> patches and provide reviews for gfortran have twindled to 1 or 2
> people
> with sporadic available time.  Did you actually review the proposed
> changes?  If not, how can you rubber stamp this commit?  You have a
> total of 12 ChangeLog entries over 18 years with the last occurring
> in
> 2011, and I do not recall you ever reviewing a patch. 

FWIW Toon reported in BZ that patch 2 in the kit fixed the ICE he had
reported, and I asked there if he was able to review this patch, which
is what led to his email.

I'm sorry if I overstepped the mark here.

>  Finally, trunk
> is in stage 4 (regression fixes & docs only).  This does not look
> like
> either.

Indeed.  The analyzer is a new feature in GCC 10.  I'm hoping some
latitude can be granted here given it's new (and hence all of its ICEs
are, strictly speaking, not regressions), and this is about adding test
coverage for fixing them.

> If I bootstrap gcc with fortran
> 
> % mkdir obj
> % ../gcc/configure --prefix=$HOME/work --enable-languages=c,fortran \
>   --enable-bootstrap --enable-checking=yes
> % gmake bootstrap
> 
> and then do
> 
> % cd gcc
> % gmake check-fortran
> 
> are these analyzer testcases built/executed? 

That's the intent of the patch, yes (the cases are marked with dg-do
compile, so "built", at least, if not "executed").

Note that it's possible to disable the analyzer at configure-time via
-fdisable-analyzer; the analyzer.exp checks for this via
check_effective_target_analyzer.

> Do all architectures
> that support gfortran also support the analyzer infrastructure?

I believe so, yes; I've fixed bugs on e.g. powerpc-ibm-aix7.2.0.0 since
merging (and if not, the check_effective_target_analyzer ought to
immediately reject running the tests).

That said, I've only verified these testcases on x86_64-pc-linux-gnu so
far.

An alternative would be to split patch 2, committing the ICE fix to the
analyzer, and leaving the test coverage to next stage 1.

Thoughts?

Thanks
Dave