[PATCH] D98086: [FileCheck] Fix numeric error propagation
Thomas Preud'homme via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 8 07:03:01 PST 2021
thopre added a comment.
I like the idea behind it. I suppose that's what you had in mind when you wrote your comments in https://reviews.llvm.org/D86222. It should make that patch a lot simpler. Thanks and sorry I didn't progress faster on that one.
================
Comment at: llvm/include/llvm/FileCheck/FileCheck.h:137-141
+ /// Indicates an error for a match found for an expected or excluded
+ /// pattern. The error is specified by \c Note, to which it should be
+ /// appropriate to prepend "error: " later. The full match itself should be
+ /// recorded in a preceding diagnostic of a different \c MatchFound match
+ /// type.
----------------
May I suggest making it clear that this error is despite the match being successful? It was not obvious to me until I read the rest of the patch.
================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:1195-1200
+ size_t MatchLen = FixedStr.size();
size_t Pos =
IgnoreCase ? Buffer.find_lower(FixedStr) : Buffer.find(FixedStr);
if (Pos == StringRef::npos)
return make_error<NotFoundError>();
+ return MatchResult(Pos, MatchLen, Error::success());
----------------
Nit
================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:2092-2094
+ int MatchedCount, StringRef Buffer, Error MatchError,
+ bool VerboseVerbose,
+ std::vector<FileCheckDiag> *Diags) {
----------------
Why the reordering of MatchError(s)?
================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:2435
MatchPos += MatchPosBuf;
- if (Req.VerboseVerbose)
- PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer, MatchPos,
- MatchLen, Req, Diags);
+ if (MatchResult.TheError || Req.VerboseVerbose) {
+ if (printMatch(/*ExpectedMatch=*/true, SM, Prefix, Pat.getLoc(), Pat, 1,
----------------
Shouldn't we delegate to printMatch to check for Req.VerboseVerbose?
================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:2492
}
if (!Req.VerboseVerbose)
+ printMatch(/*ExpectedMatch=*/true, SM, Prefix, Pat.getLoc(), Pat, 1,
----------------
Likewise.
================
Comment at: llvm/lib/FileCheck/FileCheckImpl.h:701
const FileCheckRequest &Req);
- /// Matches the pattern string against the input buffer \p Buffer
+ /// A match that possibly includes an error.
+ struct Match {
----------------
Move below above MatchResult.
================
Comment at: llvm/lib/FileCheck/FileCheckImpl.h:716
///
- /// \returns the position that is matched or an error indicating why matching
- /// failed. If there is a match, updates \p MatchLen with the size of the
- /// matched string.
+ /// \returns returns either (1) an error resulting in no match or (2) a match
+ /// possibly with an error encountered while processing the match.
----------------
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98086/new/
https://reviews.llvm.org/D98086
More information about the llvm-commits
mailing list