Page MenuHomePhabricator

[MS Demangler] Demangle data member pointers
ClosedPublic

Authored by zturner on Jul 20 2018, 10:21 PM.

Details

Summary

I intentionally didn't do member function pointers yet as I'm trying to keep the changes small and targeted.

The main trick here is being able to properly guess -- after you know you have a pointer -- whether it is a member pointer or not. You need to look for the pointee qualifier, but it is not necessarily the first character after the pointer type indicator.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner created this revision.Jul 20 2018, 10:21 PM
zturner updated this revision to Diff 156677.Jul 21 2018, 8:47 AM

I changed this slightly so that MemberPointerType is now its own derived class. This gives a little more control and flexibility over how we print them and also removes some branches from the pointer codepath, making each thing a little easier to follow.

majnemer added inline comments.Jul 21 2018, 10:49 PM
llvm/lib/Demangle/MicrosoftDemangle.cpp
252 ↗(On Diff #156677)

s/True/Non-null/

594 ↗(On Diff #156677)

Nit: Might wanna factor this condition into a little helper to ensure that pre and post are consistent.

a few nits (haven't looked at substance):

llvm/lib/Demangle/MicrosoftDemangle.cpp
302 ↗(On Diff #156677)

no break after return

307 ↗(On Diff #156677)

don't

318 ↗(On Diff #156677)

just return MangledName.front() == '8';, ternary does nothing

ping. Any objections or suggestions?

Edit: Ignore that. Apparently I had a stale copy open in my browser and didn't notice that there are, in fact, already suggestions.

zturner updated this revision to Diff 157534.Jul 26 2018, 10:57 AM

Fixed suggestions

zturner added inline comments.Jul 26 2018, 11:31 AM
llvm/lib/Demangle/MicrosoftDemangle.cpp
594 ↗(On Diff #156677)

I've done this in a followup patch which I plan to apply after I submit the member function pointer patch, since there are some other changes that make sense to do at the same time such as moving the calling convention inside the parens.

rnk accepted this revision.Jul 26 2018, 12:41 PM

lgtm

This revision is now accepted and ready to land.Jul 26 2018, 12:41 PM
This revision was automatically updated to reflect the committed changes.