[PATCH] D149707: [Demangle] remove unused status param of itaniumDemangle

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

In D149707#4316149 <https://reviews.llvm.org/D149707#4316149>, @MaskRay wrote:

> I think the signature `llvm::itaniumDemangle` was to match `__cxxabiv1::__cxa_demangle`: https://itanium-cxx-abi.github.io/cxx-abi/abi.html#demangler

Ah, that makes sense for "historically, why did this interface look like this?"

> I agree that the `status` parameter is unneeded by most users. The error categories distinguished by `status` become unuseful after the `buf` and `n` parameters were removed.
>
> This change looks good. I am unfamiliar with how LLVMDemangle code is copied to libcxxabi. Does this change affect `libcxxabi/src/cxa_demangle.cpp`? If no, I think the patch is good to go.

Good question; I believe it does not. In my change to llvm/lib/Demangle/ItaniumDemangle.cpp, you can see in the context on phab on line 363 the comment:

`// Code beyond this point should not be synchronized with libc++abi.`

(My changes are beyond that point).

libcxxabi's demangler only concerns itself with itanium style demangling, not microsoft, rust or D demangling. `llvm::itaniumDemangle` uses a `ManglingParser` which is the first point of interface into code that's synchronized between llvm/ and libcxxabi/. This change is not something that is downstream of libcxxabi (and thus has no need to be copied back upstream).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149707