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

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
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;
----------------
jhenderson wrote:
> thopre wrote:
> > jhenderson wrote:
> > > 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.
> > I'm not sure, this is copied over from PrintNoMatch. I thought it is used so that the "scanning from here" shows the first non blank line after last match but I haven't looked in details.
> Could you investigate a bit more and then update the comment with your findings? This code at the moment also skips some whitespace, which isn't always desirable (imagine if you had a regex match looking for some explicit whitespace before some string).
The code was added in da108b4ed4e6e7267701e76d5fd3b87609c9ab77 when CHECK-NEXT was added and at the same time as the "scanning from here" diagnostic was added. There was no CHECK-SAME at the time and I guess the goal was to avoid confusion by showing the last matching line (even though the column info would show it's the end of the line). There was no regular expression match either, so no way to match a newline.

I think this should simply be removed, in a separate patch. What is your opinion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86222