Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[Demangle] demangle builtin type transformations
AbandonedPublic

Authored by HerrCai0907 on Apr 16 2023, 6:55 AM.

Details

Summary

Fixed: https://github.com/llvm/llvm-project/issues/62127
https://reviews.llvm.org/D116203 introduced several compiler builtin
equivalents of the unary type traits. In some cases (e.g. template) those
builtin will be dependent and need to be mangle.
This patch add the check for u{builtin}I{type}E to demangle it.

Diff Detail

Event Timeline

HerrCai0907 created this revision.Apr 16 2023, 6:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2023, 6:55 AM
HerrCai0907 requested review of this revision.Apr 16 2023, 6:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2023, 6:55 AM

port change to libcxx

Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2023, 7:49 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
shafik added a subscriber: shafik.

Adding who I think are additional relevant reviewers.

Your test showed up as a 'binary' file? Also, I don't have code ownership in the demangler, so hopefully someone else can come along.

cjdb added a comment.Apr 20 2023, 3:19 PM

Your test showed up as a 'binary' file? Also, I don't have code ownership in the demangler, so hopefully someone else can come along.

This happened to me too. I think Phabricator assumes anything over a certain number of lines is a binary file regardless of its contents.

Can you please add a link to the github issue in the description and can this change be tested?

HerrCai0907 edited the summary of this revision. (Show Details)Apr 30 2023, 9:46 PM

can this change be tested?

Test case is in https://reviews.llvm.org/F27268964$30119

Thank you for working on this! The changes seem reasonable to me, but I'd love to hear from @rjmccall as Itanium ABI owner too.

(Should this probably have a release note?)

rjmccall accepted this revision.Thu, Sep 7, 11:59 AM

Ah, I didn't see that this was waiting on me. I'm not much of an expert in the demangler, but at a glance, this looks right.

replace StringView with std::string_view

While new reviews should be on Github, we prefer finishing existing reviews here so that we do not lose all the history of changes and conversations.