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

[Pavel Labath via Phabricator via llvm-commits]

labath 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;
----------------
rsmith wrote:
> 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?
You're right, they are orthogonal. I needed an allocator since I wanted to write a test for this (in LLVM), and I didn't want to reimplement an allocator there. Later, I realised that I can just use allocator from LLVMSupport instead and that this will not cause a dependency cycle, since it's test-only, but i didn't get around to updating the test (AFK).

I'll revert this part in the next update.


================
Comment at: src/demangle/ItaniumDemangle.h:2214
+template <typename Derived, typename Alloc = DefaultAllocator>
+struct AbstractDb {
   const char *First;
----------------
erik.pilkington wrote:
> rsmith wrote:
> > 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`)?
> Yep, I agree. Db is a really horrible name for this.
ManglingParser sounds good.


================
Comment at: src/demangle/ItaniumDemangle.h:2246
 
-  Db(const char *First_, const char *Last_) : First(First_), Last(Last_) {}
+  AbstractDb(const char* First_, const char* Last_)
+      : First(First_), Last(Last_) {}
----------------
erik.pilkington wrote:
> I think libcxxabi's clang-format moved all the pointers to the left. I just committed r344316, which overrides libcxxabi's behaviour for this directory. Can you re-run this through clang-format-diff?
Will do.


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D52992