[llvm] a33b40d - [NFC][DebugInfo] Autogenerate check lines in assignment-tracking/sroa/*

[Robinson, Paul via llvm-commits]

> > I am concerned that blindly converting to scripted checks
> > is going to lose more coverage than the one example that I
> > happened to notice by chance here.
> I have no interest in converting every (debuginfo or not) test
> to use the script. I simply do not wish to entertain the idea that
> one has to spend many tens of minutes manually updating check lines
> in tests where there are tools for that particular area of tests.
> (Aka, i'm doing this because upcoming patch touches these tests)
> 
> > What validation do we
> > have that the script is doing reasonable and correct things?
> Define reasonable and correct?

Doesn't throw away tests of debug-info metadata that the test
had originally, for one thing.

> Pretty much all LLVM tests use it.

In the areas you work on, possibly; but certainly not LLVM overall.
check-llvm reports 45,517 tests; I get 14,577 hits on the string
"Assertions have been autogenerated."  Let's be generous and
call it one-third. Not "pretty much all."

I have to say, it's relatively rarely that I encounter a test
with automated checks.  I suppose I have ever used the scripts,
but unlikely more than once or twice a year.  The areas you
work on and the areas I work on clearly don't overlap much, but
that doesn't mean your tools are correct and valid everywhere;
clearly they are not used in MOST of LLVM, and they are DEFINITELY
not prepared to handle debug-info tests.

> > I see that you modified the script; did anyone with
> > debug-info expertise review that change?
> I only told it that the metadata that is used by !DIAssignID is a
> metadata,
> that should not be hardcoded in check-output but named and checked.
> 
> Also, !DIAssignID is not documented in LangRef, it seems?

And there are many other debug-info metadata tags, which apparently
the script is not aware of?  The next time one of your patches
affects a debug-info test, how will anyone know that coverage has
been lost?  We would never have known for this patch. How much test 
coverage are you willing to throw away?

> > Please do not XFAIL any debug-info tests without filing
> > an issue first, and if you could cite the issue in a comment
> > by the XFAIL, that would be very helpful.
> Bingo! And that's precisely the problem.
> 
> If you can not rely on a script to do the update,
> can you *really* rely on the people, that have absolutely no idea
> how the debug info is represented in LLVM IR,
> to *manually* update these tests,
> more correctly than the script would?
> 
> Or should every single patch that would require
> updates to debuginfo tests be required
> to go through someone from DebugInfo team?

Until we have confidence that the scripts aren't throwing away
useful checks, like they did this time, maybe they should.

--paulr