[PATCH] D99810: [ifs] Prepare llvm-ifs for elfabi/ifs merging.
Petr Hosek via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 14 00:55:34 PDT 2021
phosek accepted this revision.
phosek added inline comments.
This revision is now accepted and ready to land.
================
Comment at: llvm/include/llvm/InterfaceStub/ELFObjHandler.h:25
namespace elfabi {
----------------
Shouldn't this be in namespace `ifs`?
================
Comment at: llvm/include/llvm/InterfaceStub/IFSStub.h:82-83
+ return true;
+}
+inline bool operator!=(const IFSTarget &Lhs, const IFSTarget &Rhs) {
+ return !(Lhs == Rhs);
----------------
Use empty line between functions.
================
Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:57-61
+ if (!Triple && !ObjectFormat && !Arch && !ArchString && !Endianness &&
+ !BitWidth) {
+ return true;
+ }
+ return false;
----------------
This can be simplified.
================
Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:65
+uint8_t elfabi::convertIFSBitWidthToELF(IFSBitWidthType BitWidth) {
+ return BitWidth == IFSBitWidthType::IFS32 ? ELF::ELFCLASS32 : ELF::ELFCLASS64;
+}
----------------
I'd consider using `switch` here so if someone adds a new entry to the `IFSBitWidthType` enum, it gets caught by the compiler.
================
Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:69
+uint8_t elfabi::convertIFSEndiannessToELF(IFSEndiannessType Endianness) {
+ return Endianness == IFSEndiannessType::Little ? ELF::ELFDATA2LSB
+ : ELF::ELFDATA2MSB;
----------------
Ditto here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99810/new/
https://reviews.llvm.org/D99810
More information about the cfe-commits
mailing list