[PATCH] D132371: [DWARFLinker][NFC] Change interface of DWARFLinker to specify accel table kinds explicitly.

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: llvm/include/llvm/DWARFLinker/DWARFLinker.h:33-37
 enum class DwarfLinkerAccelTableKind : uint8_t {
-  None,
   Apple,   ///< .apple_names, .apple_namespaces, .apple_types, .apple_objc.
-  Dwarf,   ///< DWARF v5 .debug_names.
-  Default, ///< Dwarf for DWARF5 or later, Apple otherwise.
-  Pub,     ///< .debug_pubnames, .debug_pubtypes
+  DWARFv4, ///< .debug_pubnames, .debug_pubtypes
+  DWARFv5  ///< .debug_names.
 };
----------------
So now this removes "None" and "Default" and renames "Pub" to "DWARFv4"? I think this is more confusing than it was. 

If it were me I'd say:

```
enum class DwarfLinkerAccelTableKind : uint8_t {
  Apple,
  DebugNames,  // or DWARFv5 or DWARF5
  Pub, 
```

On Apple platforms we want to use Apple for DWARF <=4 and debug names for >= 5.


================
Comment at: llvm/include/llvm/DWARFLinker/DWARFLinker.h:784
+    /// The accelerator table kinds
+    SmallVector<DwarfLinkerAccelTableKind, 8> AccelTables;
 
----------------
I would expect a value of 1 here. 


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:1818-1820
+      for (const auto &Namespace : Unit.getNamespaces())
+        AppleNamespaces.addName(Namespace.Name, Namespace.Die->getOffset() +
+                                                    Unit.getStartOffset());
----------------
Unrelated formatting changes?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132371/new/

https://reviews.llvm.org/D132371