[PATCH] D53538: NFC: Reorganize the demangler a bit

[Erik Pilkington via Phabricator via llvm-commits]

erik.pilkington added inline comments.


================
Comment at: libcxxabi/src/demangle/README.txt:51
+
+I also highly recommend running libFuzzer after non-trivial changes, see
+libcxxabi/fuzz/cxa_demangle_fuzzer.cpp and https://llvm.org/docs/LibFuzzer.html.
----------------
labath wrote:
> Use an impersonal form here? "It is also highly recommended...", or at least change "I" to "we".
Sure, fixed in the update.


================
Comment at: llvm/include/llvm/Demangle/DemangleConfig.h:75
 
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-#define LLVM_DUMP_METHOD LLVM_ATTRIBUTE_NOINLINE LLVM_ATTRIBUTE_USED
+#if !defined(NDEBUG) || defined(DEMANGLE_ENABLE_DUMP)
+#define DEMANGLE_DUMP_METHOD DEMANGLE_ATTRIBUTE_NOINLINE DEMANGLE_ATTRIBUTE_USED
----------------
andrew.w.kaylor wrote:
> Is DEMANGLE_ENABLE_DUMP ever going to be defined? LLVM_ENABLE_DUMP gets defined in llvm-config.h in response to a CMake variable. I don't see anything analogous here.
Ah, no it's not, sorry! Just went through this file too quickly I guess.


https://reviews.llvm.org/D53538