[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