[PATCH] D52992: cxa_demangle: make demangler's parsing functions overridable

Richard Smith - zygoloid via Phabricator reviews at reviews.llvm.org
Thu Oct 11 16:13:08 PDT 2018


rsmith added inline comments.


================
Comment at: src/demangle/ItaniumDemangle.h:2140-2211
+class BumpPointerAllocator {
+  struct BlockMeta {
+    BlockMeta* Next;
+    size_t Current;
+  };
+
+  static constexpr size_t AllocSize = 4096;
----------------
I'd like for us to be moving towards making this particular allocator implementation be libc++abi-specific (LLVM's use of the demangler code doesn't want it, for instance, and I suspect it's not really what LLDB wants either). This change seems to entrench the use of this allocator by giving `Db` it as a default argument, which seems like a step backwards. I also think this change is orthogonal to the rest of the patch; we don't need these classes to be in the header to convert `Db` to a CRTP base class, right?


================
Comment at: src/demangle/ItaniumDemangle.h:2214
+template <typename Derived, typename Alloc = DefaultAllocator>
+struct AbstractDb {
   const char *First;
----------------
If you're going to do a mass rename on this class anyway, have you considered giving it a more reasonable name (maybe something like `ManglingParser`)?


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D52992





More information about the libcxx-commits mailing list