[llvm] [Object] Provide operator< for ELFSymbolRef (PR #89861)
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 29 00:29:18 PDT 2024
================
@@ -1519,24 +1519,52 @@ TEST(ELFObjectFileTest, ELFSymbolRefLess) {
ASSERT_THAT_EXPECTED(ElfOrErr, Succeeded());
const ELFObjectFile<ELF64LE> &Obj = *ElfOrErr;
- llvm::object::DataRefImpl Data1;
- Data1.d.a = 0x00000000;
- Data1.d.b = 0x00000001;
- SymbolRef Symbol1(Data1, &Obj);
- ELFSymbolRef ELFSymbol1(Symbol1);
-
- llvm::object::DataRefImpl Data2;
- Data2.d.a = 0x00000000;
- Data2.d.b = 0x00000100;
- SymbolRef Symbol2(Data2, &Obj);
- ELFSymbolRef ELFSymbol2(Symbol2);
-
- // SymbolRef operator< uses std::memcmp treating the union as char string.
- if (llvm::sys::IsLittleEndianHost)
- EXPECT_FALSE(Symbol1 < Symbol2);
- else
- EXPECT_TRUE(Symbol1 < Symbol2);
-
- // ELFSymbolRef operator< compares struct fields.
+ const uint32_t Val1 = 0x00000001;
+ const uint32_t Val2 = 0x00000100;
+
+ DataRefImpl Data1, Data2, Data3, Data4;
+
+ // Symtab index
+ Data1.d.a = Data2.d.a = Val1;
+ Data3.d.a = Data4.d.a = Val2;
+
+ // Symbol index
+ Data1.d.b = Data3.d.b = Val1;
+ Data2.d.b = Data4.d.b = Val2;
+
+ SymbolRef Symbol1(Data1, &Obj), Symbol2(Data2, &Obj), Symbol3(Data3, &Obj),
+ Symbol4(Data4, &Obj);
+ ELFSymbolRef ELFSymbol1(Symbol1), ELFSymbol2(Symbol2), ELFSymbol3(Symbol3),
+ ELFSymbol4(Symbol4);
----------------
jh7370 wrote:
Nit: I think the norm in LLVM is to have each variable declared on a separate line. I think that's particularly important where it's not just a plan default construction, as IMHO, these lines are not especially easy to read.
You could probably add a helper of some kind to do this setting up in a reader-friendly manner and reducing duplication. Possible example:
```
auto MakeSymbol = [&Obj](size_t SymtabIndex, size_t SymbolIndex) {
DataRefImpl Data;
Data.d.a = SymtabIndex;
Data.d.b = SymbolIndex;
SymbolRef Sym(Data, &Obj);
return std::make_tuple(Data, Sym, ELFSymbol(Sym));
};
auto [DataRef1, SymRef1, ELFSymbolRef1] = MakeSymbol(Val1, Val1);
// Repeat for each other symbol.
```
Given the required `auto` when using the above, I renamed the final variables to indicate their types a bit clearer, but it's not really necessary. You could consider even naming them based on their relative member values i.e. something like `ELFSymHighHigh`, `ELFSymHighLow`, `ELFSymLowHigh`, `ElfSymLowLow`.
https://github.com/llvm/llvm-project/pull/89861
More information about the llvm-commits
mailing list