In D56188#1343667, @rjmccall wrote:

...although it might be reasonable to extract the method implementations on DefaultABIInfo as helper functions so that the code can be reused without requiring a particular inheritance hierarchy.

In D30882#1074822, @dexonsmith wrote:

I don't think this is quite right. I know at least make-based incremental builds wouldn't deal well with this.

LGTM, thanks!

What is the motivation for using this, over using the --filter option? I have found --filter to be substantially more general and it is easier for us to support, so my preference would be to move it to be the standard way of solving this problem.

LGTM, with one inline request, thanks!

LGTM

LGTM, this seems like a great idea!

Why don't we make this use an script we define to test the failure mode, rather than depend on an external tool, to make the test more reliable.

LGTM

Cute, I like the idea.

Ah, got it... that's a new feature to me, I am so out of the loop. :/

https://llvm.org/svn/llvm-project/llvm/trunk@282525

I think we should just wire it up with all the other tests, from lits perspective it would just be another test suite (like we do with the regular tests and the unit tests).

I believe they do all pass, but they are *not* (IIRC) wired up to run as part of LLVM's tests. We should definitely do that.

Greg, I assumed you had LLVM commit access and could land this?

I am too out of the loop on Clang development to be able to comment on the specific direction, but I will just say that I am highly in favor of adding new features in this direction. Thank you!

This isn't correct, llvm-lit is something else (an llvm specific entry point to lit that knows the object directory). That doesn't belong in the base lit project.

This looks reasonable to me, can we get a test for the behavior though?

LGTM, but can you add a section on the format to lit.pod? I know we don't have complete docs on the format, but if you could at least add section on the syntax it would be good.

There might be clients of the capture() API which might aren't expecting it to throw on error, and will need to be updated, but I agree this makes more sense.

I have no objection.

I merged this with corrections in 271608, 271610, and 272021.

I believe you are thinking of: http://reviews.llvm.org/D18185

In cases where this is trigger, wouldn't the warning always fire and the
proper way to suppress (so developers don't regularly see warnings) to
exclude that path? If so, then it seems like erroring out is ok, it just
forces resolution of that issue immediately.

Applied in r265034

LGTM, thanks for the fix and the new test coverage!

My comments:

Right, that is what I expected, something like MetricValueSignature that
clients would anticipate is a unique but otherwise meaningless signature
for some part of the test (and display accordingly, maybe by highlighting
when it changes).

I would just use addMetric for this. I see a binary signature as just
another kind of metric data, sometimes people even use the term "measure"
to the process of computing a hash signature for a binary.

I was just running the top-level suite, with lit -sv tests.

Hi Jonathan,

Hi Jonathan,

Fine by me, can you also update the 'lit' docs in CommandGuide to note this.

It doesn't look like ChildPIDs is ever added to?

Hi Eric,

Looks fine to me!

Hi Jonathan,

LGTM

LGTM

I sympathize with the use case, and in fact if you look at LLVM and Clang's
config files they have code which tries to do a trick where if it finds it
is running in the source without a lit.site.cfg, then it tries to infer the
location of the actual exec_root by seeing if you are the kind of user who
puts such things in your path. It's messy and only works for some users and
is complicated, so I'm ultimately not a big fan of that solution, but it
was the first thing I came up with.

Looks good, some nits inline. I would like to see a test or two before this goes in, though.

LGTM, although it might be good to add a note about how require_script can
be used. To be honest I don't quite understand how you will use this yet.

Sounds good!

In addition to the inline comments, could you include some tests that exercise this code?