Fix testcase to fail without D69056 and pass with

Forgot to test without the change and realized today the test still passes so it's not testing the change.

In D85422#2199400, @thopre wrote:

Actually, could you try adding a testcase? There's a test for global_status in tests/server/ui/V4Pages.py

Landed as 53229320df0e57a7b779a3dcded72fc9e6d44e01

Landed as 554f0722ae0daaab3abb7fd2205a9f00ce787cf0

Merge handleErrors in handleAllErrors below

In D86222#2251434, @jhenderson wrote:

Code mostly looks fine now, but there are still plenty of test failures being reported by the pre-merge bot. What's the situation with those currently?

In D81667#2227851, @jhenderson wrote:

LGTM, but best wait for someone else to confirm too.

Address review comments

Landed as 70b39506a1a68f16e4d59f998c6a099f98363964

In D86504#2235499, @YangZhihui wrote:

Thanks , but I don't have commit access

LGTM. I believe this sort of fix can use post-commit review (i.e. can be committed straight away).

Rebase to see if CI is now green.

Address review comments

• Address comments
• Restructure

Add testcase

A big thank you, it'll speed up incremental debug build considerably. LGTM but I'm not too well versed in the CMake aspects of making a LLVM libraries so I'll let someone else approve this change.

In D86222#2228316, @jhenderson wrote:

In D86222#2228193, @thopre wrote:

There seems to be a number of failures like due to pattern like (taken from clang/test/CodeGen/debug-info-extern-call.c):

DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: ![[RETTYPES:[0-9]+]]
DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{

I think FileCheck should forbid variable definition in a CHECK-NOT pattern. Granted with this patch we'll catch all the uses of variables defined in CHECK-NOT but we won't catch if they are not used.

I have no idea what that is even trying to do. A definition within a CHECK-NOT doesn't make sense - either the pattern will match, enabling the capture of the variable, and FileCheck to fail (because of a positive CHECK-NOT match), or it will not match, meaning the variable can't be used. In other words, I think forbidding variable definitions in CHECK-NOT makes sense. It's probably worth bringing this up on llvm-dev and cfe-dev though, to give people a head's up/ask for opinions. There may be a way this actually is useful, but if so, I don't know it.

There seems to be a number of failures like due to pattern like (taken from clang/test/CodeGen/debug-info-extern-call.c):

• Address review comments
• rename PrintNoMatch to printNoMatch to satisfy clang-tidy

Better demonstrate precision in documentation

FYI: I've only looked at a couple of the testsuite failures and they are all genuine use of undefined variable. There's 59 of them in total.

Address all remaining review comments

Add review comments

Address more review comments

Address most comments

Add example of precision in documentation

Finish implementation based on consensus reached on questions raised by the proof of concept version.

FYI, I've had the following linker error on an incremental error which disappear if I revert this commit:

Landed as 61eaf8a9eccf4edbef18f55593d1095254ed3ff5. Selected repository is incorrect so this did not close automatically.

I can reproduce the issue on the public LNT instance: go to https://lnt.llvm.org/db_default/v4/nts/137175 and click on MultiSource/Benchmarks/PAQ8p/paq8p in the run-over-run Change details and see the URL change to http.

Actually, could you try adding a testcase? There's a test for global_status in tests/server/ui/V4Pages.py

I can reproduce the issue and the patch LGTM. Thanks!

In D68061#2178756, @probinson wrote:

@rupprecht if you are still motivated to land this after so long, the only thing I see missing is to update the example from CHECK-SAME: 1{{$}} to CHECK-SAME: {{ 1$}} and then it LGTM. If not, there seems to be enough interest that someone else can land it?

LGTM with the fixed FIXME.

In D83651#2150645, @jdenny wrote:

Removed an unused parameter from printVariableDefs.

Made unit testing of getStringValue stricter.

Added unit test for printVariableDefs. This is just a basic API test and doesn't check every aspect, such as sorting. More thorough testing appears in the FileCheck utility tests. Should we replicate all that here too? What's the general rule?

In D83651#2150410, @jdenny wrote:

Extend unit tests for changes to NumericVariable.

@thopre Is that what you meant? I didn't see a way to make this specific to @LINE.

In D83651#2150335, @jdenny wrote:

In D83651#2150334, @thopre wrote:

In D83651#2149514, @thopre wrote:

Could you add an example where getStringValue() returns None?

Sorry, I meant in the testcases. Maybe something with @LINE since you mention it.

Ah, you mean the unittests, right? I keep forgetting about those for FileCheck.

In D83651#2149514, @thopre wrote:

Could you add an example where getStringValue() returns None?

LGTM too

In D83682#2149531, @jhenderson wrote:

In D83682#2149473, @MaskRay wrote:

https://lists.llvm.org/pipermail/llvm-dev/2020-July/143344.html discusses --allow-empty, but I still don't get why we need --allow-empty.

I think the idea is to prevent unexpected empty output from occurring in cases when --allow-empty is not specified. In a case where all a user cares about is that some string doesn't appear in the output, that might help make the test more robust (because they expect some output, just not what they specified), although honestly I'm not convinced, hence my proposal in the mailing list to change it to --expect-empty.

In D83682#2149473, @MaskRay wrote:

https://lists.llvm.org/pipermail/llvm-dev/2020-July/143344.html discusses --allow-empty, but I still don't get why we need --allow-empty.

Could you add an example where getStringValue() returns None?

LGTM, thanks

LGTM once you've updated the description to mention you've removed shell sign only when there's no output.

LGTM

LGTM, thanks. Strange to have the pipe at the end but if that's the agreed upon practice so be it.

LGTM with James' suggestion

In D82543#2115084, @nkbolg wrote:

Yes, there are more than 20 cases of using command without a prompt sign. It's much more than the ones that used. Do you suppose it's better to convert them all?

In D82543#2114913, @nkbolg wrote:

• [LNT][docs] docs consistency fix

Address review comment

In D79570#2092787, @thopre wrote:

In D79570#2063727, @echristo wrote:

In D79570#2062583, @thopre wrote:

Ping? Can anyone with MC experience review this? Am I missing someone as reviewer?

I'll try to get to this today.

Ping @echristo ?

Use std::to_string to test API for big values

In D82200#2105987, @jhenderson wrote:

Shouldn't there be some unit testing for this? It seems like a better testing method than relying on an UBSan build bot failure that might not manifest again in the future if some other testing were changed.