[PATCH] D86222: Fix PR46880: Fail CHECK-NOT with undefined variable

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

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?



================
Comment at: llvm/lib/Support/FileCheck.cpp:2014
+  // the next line.
+  Buffer = Buffer.substr(Buffer.find_first_not_of(" \t\n\r"));
+  FileCheckDiag::MatchType MatchTy = FileCheckDiag::MatchError;
----------------
This might be a dumb question, but I'm not following why we're skipping '\t' and ' ' here. It certainly doesn't line up with the comment above.


================
Comment at: llvm/lib/Support/FileCheck.cpp:2031-2046
+  // Print undefined variable diagnostics.
+  MatchErrors =
+      handleErrors(std::move(MatchErrors), [&](const UndefVarError &E) {
+        SmallString<256> Msg;
+        raw_svector_ostream OS(Msg);
+        OS << "uses undefined variable: ";
+        E.log(OS);
----------------
Can't these two be folded into a single `handleAllErrors` call? IIRC, `handleAllErrors` takes any number of handler lambdas, and iterates through them until one matches, or it runs out. Thus the above code should look like the something like the suggested edit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86222/new/

https://reviews.llvm.org/D86222