[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