[PATCH] D44668: [demangler] WIP: Add a "partial" demangling API for LLDB

[Greg Clayton via Phabricator via llvm-commits]

clayborg added a comment.

Seems like it might be nice to replace all "char *Buf, size_t *N" arguments, where "char *" is returned with a llvm::raw_ostream and let the user use what ever backing / memory allocation scheme they want instead of forcing re-allocations? Probably best to include a person that has contributed to this file in the LLVM community so they can comment on the changes here.



================
Comment at: llvm/include/llvm/Demangle/Demangle.h:42
+  /// \return true on error, false otherwise
+  bool partialDemangle(const char *MangledName);
+
----------------
Use "llvm::StringRef MangledName" instead of "const char *"?


================
Comment at: llvm/include/llvm/Demangle/Demangle.h:46
+  /// second and third parameters to itaniumDemangle.
+  char *finishDemangle(char *Buf, size_t *N) const;
+
----------------
Might be better to use a "llvm::raw_ostream &out" instead of just a plain buffer? Seems like llvm::raw_ostream can be backed by what ever kind of buffer you want it to be backed (llvm::raw_string_ostream is a nice thing to use with this that uses a std::string as the destination buffer).


================
Comment at: llvm/include/llvm/Demangle/Demangle.h:54
+  /// "a::b".
+  char *getFunctionContextName(char *Buf, size_t *N) const;
+
----------------
Maybe rename to getFunctionDeclContext? or getFunctionDeclContextName?


================
Comment at: llvm/include/llvm/Demangle/Demangle.h:64
+  /// know this if the function has a cv or reference qualifier.
+  bool isProvablyMemberFunction() const;
+
----------------
s/provably/probably/ in comment and function name. Maybe "couldBeMemberFunction" might be a better name?


================
Comment at: llvm/include/llvm/Demangle/Demangle.h:69
+
+  /// If this name is a special name.
+  bool isSpecialName() const;
----------------
Can you document what a special name is here?


Repository:
  rL LLVM

https://reviews.llvm.org/D44668