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

[Luís Ferreira via Phabricator via llvm-commits]

ljmf00 added inline comments.


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


================
Comment at: llvm/lib/Demangle/DLangDemangle.cpp:44
+
+  if (Decl.getCurrentPosition() > 0) {
+    Decl << '\0';
----------------
jhenderson wrote:
> 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?
Sorry, I forgot it. Thanks for the reminder!


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

https://reviews.llvm.org/D111414