Removed config.test_exec_root, which is unique to each config by default.

In D49249#1161704, @morehouse wrote:

The other test configs don't define config.test_exec_root at all. Do we need it?

Rebased. There's 1 new test requiring -allow-deprecated-dag-overlap.

In D47106#1157397, @probinson wrote:

The mechanical changes to the tests are actually pretty easy to skim through, given the way Phabricator's web interface does change highlighting. The three you cited are all fine.

In D47106#1156353, @probinson wrote:

Given that you feel so very strongly about it, and that this is a tool that really has absolutely zero exposure outside our own test system, I'm willing to say that erring on the side of extra testing is acceptable here.

This updates -v to make more sense in the case of the new CHECK-EMPTY directive. Without this update, -v reported the match for CHECK-EMPTY as occurring at the preceding newline. That's confusing, especially if the preceding line was also empty because it then seemed like the expected empty line was the preceding line instead. Because CHECK-EMPTY is conceptually a CHECK-NEXT for an empty line, -v now reports the match as occurring after the preceding newline, just as it does for CHECK-NEXT. That is, as for CHECK-NEXT, the match range starts at the empty line, and there is a constraint that there is one preceding newline in the search range.

In D47106#1110343, @jdenny wrote:

In D47106#1110240, @probinson wrote:

At that point, I think the behavior becomes tractable. The first DAG group finds what it finds, the NOT forces no reordering therefore the second DAG group's range starts at the endpoint of the first group's matches. After the second DAG group runs, the NOT's range is from the endpoint of the first group's matches to the earliest of the second group's matches.

How does that all sound?

The main question remaining for me is about the start of the second DAG group's match range and thus whether reordering detection happens.

In D47106#1150943, @probinson wrote:

Regarding the DAG-NOT-DAG reordering diagnostic: Rereading the comments, and the llvm-dev discussion, are we agreed that we can eliminate it? That is, the first DAG group defines some match range, the second DAG group defines a disjoint match range, and the NOT group looks at the text from the end of the first match range to the start of the second match range. No attempt to diagnose ordering problems.

Ping. Rebased.

Rebased.

Rebased.

Rebased. There are 5 new tests requiring -allow-deprecated-dag-overlap.

Rebased. The number of tests broken by this change in functionality continues to grow. Can we move forward?

%T should also be deprecated (or removed) below, where it's documented incorrectly:

Rebased. Ping.

Rebased. Two more tests are affected:

Rebased.

Rebased. Ping.

Would there be any harm in committing this patch as is? If we decide to remove all support for windows cmd.exe, that should be a separate patch anyway.

In D44598#1115251, @jdenny wrote:

In D44598#1115242, @delcypher wrote:

A high level question. Why are we even testing using cmd.exe with an external shell? IIRC that's not a use case LLVM uses for Windows. Instead
the internal shell is used on Windows. Is this (cmd.exe as an external shell) really something we want to support? I don't see anything desirable about supporting cmd.exe as an
external shell because that means it can be very easy to write non-portable shell tests.

I'm not a windows developer. I fixed that because others reverted my patch when it broke windows buildbots. Someone else will have to answer. Here's the bugzilla, in case that helps:

https://bugs.llvm.org/show_bug.cgi?id=37239

In D44598#1115242, @delcypher wrote:

In D44598#1115233, @jdenny wrote:

In D44598#1115229, @lebedev.ri wrote:

In D44598#1115221, @jdenny wrote:

Ping.

What seems to be the problem?
This was accepted before, reverted due to windows bot breakage,
https://reviews.llvm.org/D44598#1091098 suggests that breakage is fixed.
Just commit again?

There was enough of a code change that I thought the reviewer would want to take another look. Is that not the right approach?

A high level question. Why are we even testing using cmd.exe with an external shell? IIRC that's not a use case LLVM uses for Windows. Instead
the internal shell is used on Windows. Is this (cmd.exe as an external shell) really something we want to support? I don't see anything desirable about supporting cmd.exe as an
external shell because that means it can be very easy to write non-portable shell tests.

In D44598#1115229, @lebedev.ri wrote:

In D44598#1115221, @jdenny wrote:

Ping.

What seems to be the problem?
This was accepted before, reverted due to windows bot breakage,
https://reviews.llvm.org/D44598#1091098 suggests that breakage is fixed.
Just commit again?

Ping.

In D47106#1110240, @probinson wrote:

(Replying here instead of inline as the thread is getting kind of long.)

Sorry, I didn't realize FileCheck would try to detect and explicitly diagnose reordering DAG across NOT. That seems like more trouble than it's worth. NOT is documented to enforce ordering, and we can make it just do that, and if you mess up then your test fails by not matching rather than telling you something is in the wrong order. (Then you can use -v or -vv to work out what happened, if you need to.)

Once we define the DAGs as separated into distinct groups by a NOT, I think it's conceptually straightforward to say each DAG group has a "matching range" from the start of the earliest match to the end of the latest match. Two DAG groups can't have overlapping match ranges (just like you are making individual DAG matches not overlap).

This splits the test case, as requested.

In D47114#1109821, @probinson wrote:

LGTM

If this doesn't accomplish the changes you had in mind, please let me know.

In D46919#1107296, @rsmith wrote:

In D46919#1101268, @jdenny wrote:

In D46919#1100493, @rsmith wrote:

The deprecated enumerator is also referenced by tools/c-index-test/c-index-test.c

This test prompted me to keep the IncludeTagDefinition member in PrintingPolicy so that clang_PrintingPolicy_getProperty would return the previous value set by clang_PrintingPolicy_setProperty. Otherwise, the value doesn't have any effect. Is that self-consistency not worth worrying about? If so, I'll remove both.

I don't think it's worth worrying about. We don't guarantee that the values round-trip in general (most of them are unsigneds being written to a bool, so we don't preserve values that are neither 0 nor 1).