[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington marked 4 inline comments as done.
erik.pilkington added inline comments.


================
Comment at: src/cxa_demangle.cpp:44
 
+class string_ref
+{
----------------
mehdi_amini wrote:
> If this is supposed to be *the* ultimate LLVM demangler, can we follow LLVM coding standard?
I would like if this followed LLVM conventions too, but this file is already written following this style and leaving it in some middle state would be ugly. All of libcxx[abi] follows this convention too, so this isn't a problem that is isolated to this file.


================
Comment at: src/cxa_demangle.cpp:239
+    virtual void print_left(stream& s) const = 0;
+    virtual void print_right(stream&) const {}
+
----------------
mehdi_amini wrote:
> Why is one pure virtual and not the other? Why isn't the stream const?
The second isn't pure virtual because its only implemented by nodes that have a part that goes on the right of the declarator, such as array types or functions types, but every node should implements print_left. The stream is where the AST is printed into, so making it const doesn't make sense. I renamed it to output_stream and wrote a comment to make this more clear in the new patch.


https://reviews.llvm.org/D35159