[PATCH] D150797: Turn unreachable error into assert
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun May 21 23:57:45 PDT 2023
jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.
LGTM, with one nit.
================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:148-150
bool MissingFormPrefix = AlternateForm && !StrVal.consume_front("0x");
+ (void)MissingFormPrefix;
+ assert(!MissingFormPrefix && "missing alternate form prefix");
----------------
thopre wrote:
> jhenderson wrote:
> > I'd get rid of the unnecessary temporary and just do the calculation inline.
> >
> > Also, is this the right place for the assert? Previously the error was later in the code.
> Will the consume_front() happen if it's inside the assert though? Because under normal circumstances (i.e. assert does not trip) the consume_front() *must* happen for the function to do the right thing.
Bleh, I missed that `consume_front` has side effects beyond getting the return value. Okay, I can't think of any real improvements (other than I think `static_cast` is preferred generally to C-style casts).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150797/new/
https://reviews.llvm.org/D150797
More information about the llvm-commits
mailing list