[Type Uniquing Patch] Use identifier instead of MDNode for getClassType of ptr_to_member.

[David Blaikie]

Looks generally reasonable.

I take it you're using pointer_to_member's base type as the first
field example & we'll see similar, smaller patches (not adding any
infrastructure, just porting another field over) for each field from
here on out? It seems like a nice, small example - thanks for that.

lib/IR/DebugInfo.cpp
 - DIDerivedType::Verify
   * we should have a single function that does
"fieldIsMDNode/fieldIsMDString" because we're going to use that test
for every type field
 - generateDITypeIdentifierMap
   * any reason this wouldn't be a ctor of DITypeIdentifierMap (or at
least a member function)?
   * "DICompositeType Ty = DICompositeType(x);" can be rewritten as
"DICompositeType Ty(x);"
   * map lookup and insertion should be combined, probably simply like this:
     pair<iterator, bool> p = Map.insert(make_pair(TypeId, Ty)); //
try to insert
     if (!p.second && !Ty.isForwardDecl()) // if we didn't insert
because the element was already there
       p.first->second = Ty; // replace whatever we had (declaration
or definition) with a definition
     (pseudocode, comments for illustration only)

DwarfDebug.cpp:
  - lookupType
    * assert should have an explanatory string in it

'getClassType' - this is going to become pervasive in every DI*
function that returns DIType, right? I think it might be helpful to
add an opaque type here so we're not just referring to any Value.
Either something totally opaque, a class that befriends the
DIDescriptor class (for construction) and the map (for lookup) &
otherwise has no public members except simple copying, etc. Or perhaps
a PointerUnion of MDNode/MDString - this is unnecessary, of course,
since the Value hierarchy already knows which object it is, so there's
no need to discriminate the pointer in actuality...

On Fri, Aug 30, 2013 at 10:40 AM, Manman Ren <manman.ren at gmail.com> wrote:
> Also add a typedef for DITypeIdentifierMap and a helper
> generateDITypeIdentifierMap in DebugInfo.h. In DwarfDebug.cpp, we keep
> a DITypeIdentifierMap and call generateDITypeIdentifierMap to actually
> populate the map. DwarfDebug also has a helper function lookupType that
> finds the type MDNode for a given Value (either an identifier in MDString
> or the actual MDNode).
>
> Verifier is updated accordingly.
>
> 3 patches are attached:
> The first one is a simple refactoring to add getField to DIDescriptor so we
> can use it to return Value* for getClassType.
>
> The second one is the actual change to DIBuilder and DwarfDebug etc with an
> added testing case.
>
> The last one is to update testing cases in clang.
>
> Thanks,
> Manman
>