[PATCH] D76977: [lld-macho] Implement basic export trie

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/MachO/ExportTrie.cpp:11
+// Documentation of the format can be found in
+// llvm/tools/obj2yaml/macho2yaml.cpp.
+//
----------------
I'd add a file comment to explain the structure of the export table, that is, the symbol table is a trie instead of the usual runs of null-terminated strings. So it can be prefix-compressed and more compact, though it's more complicated.




================
Comment at: lld/MachO/ExportTrie.cpp:48
+struct TrieNode {
+  typedef ilist<TrieEdge> TrieEdgeList;
+
----------------
I wonder if you can use std::vector.


================
Comment at: lld/MachO/ExportTrie.cpp:52
+      : cumulativeString(s), address(0), trieOffset(0), hasExportInfo(false) {}
+  ~TrieNode() = default;
+
----------------
Do you need this destructor?


================
Comment at: lld/MachO/ExportTrie.cpp:66
+  TrieEdgeList children;
+  uint64_t address;
+  // Estimated offset from the start of the serialized trie to the current node.
----------------
Initialize with `= 0` here.


================
Comment at: lld/MachO/ExportTrie.cpp:75
+
+void TrieNode::addSymbol(const Symbol &entry,
+                         std::vector<TrieNode *> &allNodes) {
----------------
`addSymbol` seem to keep the internal trie consistent all the time, but I don't think we need to split the work between `addSymbol` and `build`. We can make `addSymbol` just to store an argument to an array, and move the code for trie construction to `build`.


================
Comment at: lld/MachO/ExportTrie.h:23
+  void addSymbol(const Symbol &sym);
+  llvm::SmallVector<char, 128> build();
+
----------------
I don't think we need to be efficient here. I'd use std::vector<> instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76977