[PATCH] D62753: [llvm-lipo] Implement -archs
Alexander Shaposhnikov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 31 17:49:16 PDT 2019
alexshap added inline comments.
Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:154
+ llvm_unreachable("llvm-lipo action unspecified");
> alexshap wrote:
> > smeenai wrote:
> > > compnerd wrote:
> > > > I don't think that this should be unreachable - if the action is unspecified, that can occur normally in the way that the user invokes the tool. Perhaps this should be a proper error?
> > > Lines 123 through 133 should ensure we have a single action argument of the expected type. My switch statement suggestion above should make that structure clearer.
> > llvm_unreachable is for internal logical errors, here if the specified command line arguments are incorrect we should make use of reportError, the tool should not crash.
> I actually think this would be an internal logic error:
> * If the user hasn't specified exactly one action argument, we'll have errored out already.
> * The only action arguments we currently support are `-verify_arch` and `-archs`.
> * Therefore, if our option isn't one of them, something is pretty wrong with our program logic.
> I'm okay with using `reportError` for this though.
imo it's not. llvm_unreachable wouldn't be the right choice here for the following reason: for example, if a user has misspelled a flag or option the tool should not crash with assert/llvm_unreachable, it should exit normally (report error and exit with non-zero code)
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the llvm-commits