dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM once you clean up the `#if 0`.



================
Comment at: src/cxa_demangle.cpp:260-261
+
+#if 0
+  void dump() const {
+    char *Buffer = static_cast<char*>(std::malloc(1024));
----------------
erik.pilkington wrote:
> dexonsmith wrote:
> > Why is this behind `#if 0`?  Should you just use something like 
> > `LLVM_DUMP_METHOD`?
> Do you mean conditionally defined based on NDEBUG and marked 
> __attribute__((noinline,used)) for debuggers? I was just using it as a quick 
> & simple little helper for debugging.
Yup, those two things sound great to me.  You might need some platform-specific 
logic to define the attributes correctly.

I figured it was a quick and simple helper, like the `dump()` methods 
throughout LLVM.  Here's why I think you should change it:
- I imagine this will be useful in the future, too, and being able to call 
`dump()` from the debugger without recompiling is a great feature.
- `#if 0`s render as comments in some editors, and then bitrot because no one 
compiles them.


https://reviews.llvm.org/D41885



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to