[llvm] [FileCheck] Handle CHECK-EMPTY at EOF without trailing newline (PR #196576)
Joel E. Denny via llvm-commits
llvm-commits at lists.llvm.org
Sat May 9 13:20:12 PDT 2026
================
@@ -1881,17 +1903,21 @@ bool FileCheck::readCheckFile(
assert(UsedPrefix.data() == Buffer.data() &&
"Failed to move Buffer's start forward, or pointed prefix outside "
"of the buffer!");
+
+ const char *BufferEnd = Buffer.data() + Buffer.size();
assert(AfterSuffix.data() >= Buffer.data() &&
- AfterSuffix.data() < Buffer.data() + Buffer.size() &&
+ AfterSuffix.data() <= BufferEnd &&
"Parsing after suffix doesn't start inside of buffer!");
// Location to use for error messages.
const char *UsedPrefixStart = UsedPrefix.data();
// Skip the buffer to the end of parsed suffix (or just prefix, if no good
// suffix was processed).
- Buffer = AfterSuffix.empty() ? Buffer.drop_front(UsedPrefix.size())
- : AfterSuffix;
+ bool AfterSuffixAtEOF = AfterSuffix.data() == BufferEnd;
+ Buffer = (AfterSuffix.empty() && !AfterSuffixAtEOF)
+ ? Buffer.drop_front(UsedPrefix.size())
+ : AfterSuffix;
----------------
jdenny-ornl wrote:
With or without this PR, I do not understand the "then" of this conditional:
1. How is `Buffer = Buffer.drop_front(UsedPrefix.size())` ever the right thing to do here? Wasn't the prefix already removed by `FindFirstMatchingPrefix`'s call to `FindCheckType`? If it wasn't and we remove it this way, doesn't that leave at least the directive's trailing `:`? How is that ever correct for the remaining code?
2. How is `AfterSuffix.empty()` ever true here? If I add `assert(!AfterSuffix.empty());` here, FileCheck's existing test suites pass.
3. OK, `AfterSuffix.empty()` is true for the case addressed by this PR. But this patch chooses the "else" for that case.
Does anyone see something I'm missing?
If not, instead of complicating useless logic further, let's just replace all that with `Buffer = AfterSuffix;` and fix the comment.
https://github.com/llvm/llvm-project/pull/196576
More information about the llvm-commits
mailing list