[PATCH] D111414: [Demangle] Add minimal support for D programming language

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 8 15:08:23 PDT 2021


dblaikie added inline comments.


================
Comment at: llvm/lib/Demangle/DLangDemangle.cpp:36
+/// \note Beware these aren't required to be '\0' terminated
+struct OutputString {
+
----------------
Any chance of using an existing stream type (like `llvm::itanium_demangle::OutputStream` which `RustDemangle` also uses?)? Otherwise might be worth a bunch of separate testing of this class - or incrementally adding functionality to it that functionality is used in the patch series, otherwise it's hard to tell that everything's tested if it's added in one go here & significant parts are currently unused.


================
Comment at: llvm/unittests/Demangle/DLangDemangleTest.cpp:19
+
+TEST(DLangDemangle, Success) {
+  struct ExpectedVal {
----------------
I think most of the mangling testing is done via llvm-cxxfilt - see llvm/test/Demangle/rust.test - so probably best this testing be done that way too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111414/new/

https://reviews.llvm.org/D111414



More information about the llvm-commits mailing list