[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