[PATCH] D95654: ADT: Sink the guts of StringMapEntry::Create into StringMapEntryBase

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Sounds good - some idle optional thoughts.



================
Comment at: llvm/include/llvm/ADT/StringMapEntry.h:30-34
+
+protected:
+  template <typename AllocatorTy>
+  static void *allocateWithKey(size_t EntrySize, size_t EntryAlign,
+                               StringRef Key, AllocatorTy &Allocator);
----------------
Took me a minute to figure out why this was a member function - since it doesn't need access to any members, etc. Perhaps it could be a free function in a detail or implementation namespace? Not sure if we have a strong convention of how that's done in LLVM.


================
Comment at: llvm/include/llvm/ADT/StringMapEntry.h:48-55
+  assert(Allocation && "Unhandled out-of-memory");
+
+  // Copy the string information.
+  char *Buffer = reinterpret_cast<char *>(Allocation) + EntrySize;
+  if (KeyLength > 0)
+    ::memcpy(Buffer, Key.data(), KeyLength);
+  Buffer[KeyLength] = 0; // Null terminate for convenience of clients.
----------------
Guess this half could be pulled out into a non-template function, but maybe that doesn't tilt the inliner enough & hurts performance?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95654