RFC: Monitoring old PRs, new dg directives

[Marek Polacek]

On Fri, Aug 07, 2020 at 09:21:27AM -0400, Nathan Sidwell wrote:
> On 8/6/20 6:55 PM, Marek Polacek wrote:
> > On Thu, Aug 06, 2020 at 10:01:37AM -0400, Nathan Sidwell wrote:
> > > On 8/5/20 7:29 PM, Marek Polacek wrote:
> > > > On Wed, Aug 05, 2020 at 11:03:08AM -0400, Nathan Sidwell wrote:
> > > > > On 8/4/20 8:54 PM, Marek Polacek via Gcc-patches wrote:
> > > > > > On Tue, Aug 04, 2020 at 03:33:23PM -0700, Mike Stump wrote:
> > > > > > > I think the read of the room is that people think it would be generally useful, so let approve the general plan.
> > > > > > 
> > > > > > Cool.
> > > > > > 
> > > > > > > So, now we are down to the fine details.  Please do see just how far you can stretch the existing mechanisms to cover what you need to do.  I think the existing mechanisms should be able to cover it all; but the devil is in the details and those matter.
> > > > > > 
> > > > > > At this point I'm only proposing one new directive, dg-ice.  I think we can't
> > > > > > really do without it.  The other one was a matter of convenience.
> > > > > 
> > > > > I've realized I have a concern.  Grepping (or searching in an editor buffer)
> > > > > the log file for 'internal compiler error' to find actual regressions is a
> > > > > thing I want to still be able to do (perhaps with alternative spelling, I
> > > > > don't care).  I don't want to see the ICEs of tests that are expected to
> > > > > ICE.
> > > > > 
> > > > > I think that means there has to be a positive marker on the unexpected ICEs,
> > > > > rather than lack of an expected marker on them.
> > > > 
> > > > Hmm, by the log file you mean g++.log?  Currently, if you run a dg-ice test,
> > > > and the test still ICEs, the g++.log file (but not the stdout of make
> > > > check-c++!) will have:
> > > > 
> > > > Executing on host: ... xg++ with options ...
> > > > spawn -ignore SIGHUP ... xg++ with options ...
> > > > .../foo.C:14:15: internal compiler error: in poplevel_class, at cp/name-lookup.c:4225
> > > > <backtrace>
> > > > compiler exited with status 1
> > > > XFAIL: g++.dg/foo.C  -std=c++17 (internal compiler error)
> > > > PASS: g++.dg/foo.C  -std=c++17 (test for excess errors)
> > > > 
> > > > Which one of these would you not like to see?
> > > 
> > > Neither of these is solving the issue.  How do I find the ICES that are
> > > unexpected, without tripping over the ICEs that are expected?
> > > 
> > > > Can you give me more details?  Hopefully we'll work something out that doesn't
> > > > break your workflow.
> > > 
> > > sure.
> > > * develop patch
> > > * run testsuite
> > > * observe unexpected ICEs
> > > * load g++.log into editor
> > > * ^sinternal comp
> > > * gets to first unexpected ICE
> > > * debug it.
> > > 
> > > What does '^sinternal comp' become?  As there could be many expected ICEs
> > > it'll be painful to determine whether any particular utterance of 'internal
> > > compiler' is expected or not.
> > 
> > That is a problem I don't know how to deal with.  I know how to pass
> > additional options to the compiler from dejagnu.  I thought maybe I could use
> > -pass-exit-codes, redirect stderr to /dev/null, and check if the exit code is
> > ICE_EXIT_CODE, but there seems to be no way to do that redirection.  So I'm
> > stuck.
> > 
> > Though, you could just grep for '^FAIL.*internal comp', which will find the
> > first unexpected ICE.  Contrary to the expected ICEs, the unexpected ICEs will
> > be shown in 'Excess errors:'.  Won't that work?
> 
> Read what I wrote:
> >> * load g++.log into editor
> >> * ^sinternal comp
> 
> are you telling me not to use an editor for this task?  the search is so one
> can get the command line.  Grepping the log file will not do that.

No, I'm saying that looking for '^FAIL.*internal comp' (in an editor) instead
of just 'internal comp' will get you to the ICEs you care about.  In vim,
'/^FAIL.*in' gets me there.

If that is still too much, we could maybe add a new diagnostic kind, like
DK_EXPECTED_ICE that has different wording than DK_ICE, and have dg-ice
use it via some option/envvar.

> while solveable, this seems to be the equivalent of putting stones in ones
> shoe.  an annoyance one could do without.  I'm sadly not convinced the goal
> is worth it.

If this is an annoyance to people, I'll just go back to my own repo.

Marek