[libcxx-commits] [PATCH] D104366: [Demangle][Rust] Parse non-ASCII identifiers

Fri Oct 1 12:59:48 PDT 2021

dblaikie accepted this revision.
dblaikie added a comment.

In D104366#3037259 <https://reviews.llvm.org/D104366#3037259>, @tmiasko wrote:

> In D104366#3037010 <https://reviews.llvm.org/D104366#3037010>, @dblaikie wrote:
>
>> Yeah, I'm fairly comfortable approving a change like this - but now that you mention the libc++abi surface area here: Are other features of Utility.h tested (unit tested?) somewhere? Could you add testing for this new function too?
>>
>> Also, admittedly I don't fully understand how this LLVM change ended up needing a change to libc++abi - how do these pieces connect/depend on each other?
>
> I added a unit test for OutputStream::insert. It is also indirectly tested by the Rust demangler of course.
>
> libcxxabi carries its own copy of Itanium demangler to implement __cxa_demangle. Those changes are just keeping those two in sync.

Ah, so the issue is that the itanium demangling support is intended to be kept identical between these two places - and that includes a utility header which is, on the LLVM side, used by Rust demangling (which doesn't exist in libcxxabi) - so it's an update to this common header, which is to be kept in sync though there's no (current) use for the extra functionality on the libcxxabi side, because it's not used by the itanium demangling, only the rust demangling. (but good to keep it in sync anyway, in case someone does end up using it in itanium demangling, etc)

Fair enough. Thanks for the help!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104366