[PATCH] D99981: [demangler] Support the new Rust mangling scheme (v0)

[Tomasz Miąsko via Phabricator via llvm-commits]

tmiasko added a comment.

Thanks for suggestions @dblaikie, @jhenderson. I will create a separate preparatory patch that should be easier to a review.



================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:25-29
+  if (MangledName == nullptr || (Buf != nullptr && N == nullptr)) {
+    if (Status != nullptr)
+      *Status = demangle_invalid_args;
+    return nullptr;
+  }
----------------
dblaikie wrote:
> jhenderson wrote:
> > FWIW, I don't think there's a way to test this block without unit-testing, but I'm fine if the bulk wants to be in the lit tests. Same goes with a number of the other potential failure paths.
> Yeah, API usage issues like this should be exercised by a unit test.
> 
> Though I wonder if it might be simpler to not support this and instead assert that the parameters are non-null as needed, rather than that the caller would get some error handling result.
The main motivation behind the current API choice was to facilitate further integration. Places where I would like to eventually support Rust demangling use itaniumDemangle for the most part, so I settled on the same API. The microsoftDemangle has similar API as well. 

This API does unfortunately require a bit of extra code and unit tests to cover it (tests are in RustDemangleTest.cpp).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99981