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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Two nits still.



================
Comment at: llvm/lib/Demangle/DLangDemangle.cpp:33
+  OutputBuffer Demangled;
+  if(!initializeOutputBuffer(nullptr, nullptr, Demangled, 1024))
+    return nullptr;
----------------
Nit: please run clang-format on the latest changed code.


================
Comment at: llvm/lib/Demangle/DLangDemangle.cpp:44
+
+  if (Decl.getCurrentPosition() > 0) {
+    Decl << '\0';
----------------
jhenderson wrote:
> ljmf00 wrote:
> > jhenderson wrote:
> > > Perhaps add a comment or probably put this in a separate function, as it feels like a different level of abstraction to the rest of the code in this fucntion, and I don't really understand why it's necessary (NB: I haven't looked into the OutputString interface, but ideally, I shouldn't have to).
> > OutputString/OutputBuffer is not null terminated therefore this is necessary. This is also being done on other demanglers to make the API conformat with null terminated strings. I don't belive such tiny thing need a separate function, but I can generalize this if it is really needed.
> The separate function thought was mostly a way of describing what this code is doing, as it's not immediately clear if you aren't familiar with the `OutputBuffer` interface. A simple comment at the start of the if is probably sufficient, if you prefer that though.
Looks like this requested comment is still missing?


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

https://reviews.llvm.org/D111414