[PATCH] D19236: Add DITypeIndex for CodeView and emit it for locals

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 18 15:26:02 PDT 2016


> On 2016-Apr-18, at 14:20, Reid Kleckner <rnk at google.com> wrote:
> 
> rnk added a comment.
> 
> In http://reviews.llvm.org/D19236#404416, @llvm-commits wrote:
> 
>> Maybe "ImmutableMetadata"?
> 
> 
> I think the main property they share is that they have no metadata operands: they are leaves of the metadata graph. So, maybe LeafMetadata? MDLeaf? DILeaf?

ValueAsMetadata also has no metadata operands, but it does have
a non-Metadata operand.

Not `DILeaf`, because `MDString` isn't debug-info specific.

I don't really like LeafMetadata or MDLeaf, but I'm not sure they
are any worse than ImmutableMetadata.  I'll let you know if I come
up with something better, I mainly just care that it works.

> ================
> Comment at: unittests/IR/MetadataTest.cpp:1407
> @@ -1406,3 +1406,3 @@
>   unsigned Line = 2;
> -  DISubroutineType *Type = getSubroutineType();
> +  DITypeRef Type = DITypeRef(getSubroutineType());
>   bool IsLocalToUnit = false;
> ----------------
> dblaikie wrote:
>> X y = X(z); -> X y(z); perhaps?
>> 
>> and/or should DITypeRef be implicitly convertible from a DIType?
> The primary TypedDINodeRef constructor is marked explicit, so I shied away from adding an implicit constructor. However, it is constructing from a generic Metadata*, and an implicit constructor from T* would be pretty safe.

With `MDString`-based type references, this constructor should
*definitely* be explicit.  With the string-based references, you
have two ways of creating a `DITypeRef` from a type:

 1. Call `DITypeRef::get(T*)`.  If the argument dyn_casts to a
    `DICompositeType*` that has an 'identifier:' field, it will
    initialize with `DICompositeType::getRawIdentifier()`.
    Otherwise, it uses the argument.

    This is supposed to be the "normal" initialization.

 2. Call `DITypeRef::DITypeRef(T*)`.  This will use the argument
    directly.

    This is effectively a "bypass" mode.

The code here should probably call `DITypeRef::get()` to match
the model.  Ugly, but maybe temporary...

Once we get rid of string-based references, then I'd be fine
with changing the constructor to implicit (since it's no longer
bypassing any magic initialization).

> 
> http://reviews.llvm.org/D19236
> 
> 
> 



More information about the llvm-commits mailing list