[PATCH] D63735: [MachOObjectFile]Added Valid Architecture Function
Michael Trent via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 24 15:37:59 PDT 2019
mtrent accepted this revision.
mtrent added inline comments.
================
Comment at: llvm/include/llvm/Object/MachO.h:574
static bool isValidArch(StringRef ArchFlag);
+ static ArrayRef<StringRef> getValidArchs();
static Triple getHostArch();
----------------
Super duper nitty nit: consider "getKnownArchs" instead of "getValidArchs". I'm wrestling with the idea of what makes an arch "valid", considering the list supported here is only the subset of archs that llvm knows about. E.g., is m68k an invalid Mach-O file? And so on. I readily concede this is the most pedantic of concerns.
I can see how an argument can be invalid, and so have no concerns over llvm-lipo's validateArchitectureName function name.
================
Comment at: llvm/tools/llvm-lipo/llvm-lipo.cpp:99
+ raw_string_ostream OS(Buf);
+ OS << "Invalid architecture: " << ArchitectureName
+ << "\nValid architecture flags are:";
----------------
super duper nitty nit from above applies here, too: consider "Unknown architecture" instead of "Invalid architecture."
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63735/new/
https://reviews.llvm.org/D63735
More information about the llvm-commits
mailing list