[PATCH] D150797: Turn unreachable error into assert

Thomas Preud'homme via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 22 09:02:38 PDT 2023


thopre added inline comments.


================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:148-150
   bool MissingFormPrefix = AlternateForm && !StrVal.consume_front("0x");
+  (void)MissingFormPrefix;
+  assert(!MissingFormPrefix && "missing alternate form prefix");
----------------
jhenderson wrote:
> 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).
I agree in the general case but there is not a single use of static_cast<void> to indicate a function return value being ignored in the llvm/ subtree. There is however plenty of (void) C-style cast example. Ok to land as is?


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