[PATCH] D123420: [demangler] Rust demangler buffer reuse

[Nathan Sidwell via Phabricator via llvm-commits]

urnathan added a comment.

In D123420#3440296 <https://reviews.llvm.org/D123420#3440296>, @tmiasko wrote:

> The API thus far was intended to match that of `__cxa_demangle`, where the caller retains the ownership of the provided buffer when demangling fails. This is no longer the case in the new implementation. I would rather avoid diverging from `__cxa_demangle` in such a subtle way.

Ah, it took me a while to figure out where that's happening, I see it now.  Awkward.   I suppose the rust demangler could always return the new buffer (and free any incoming buffer) in the success case?  Something like `if (Buf) std::free(Buf);` instead of that memcpy/free dance?

> The Itanium demangler gets away with the direct use of the buffer because it separates fallible parsing from infallible printing.

Any thoughts about doing similar in the Rust demangler -- passing an output buffer reference around or something?

>> the length of the incoming buffer will have come from a previous call, which was the length of the demangled string
>
> I think this is a bug in the implementation. The *N should have been updated to reflect allocated memory size. From Demangler API in Itanium C++ ABI:
>
>> In either case, the new buffer size will be stored in *n.

Indeed, that's the direction I'm trying to go in :)

FWIW, this is part of a series trying to make buffer ownership transfer clearer (and other issues)


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

https://reviews.llvm.org/D123420