[llvm-commits] [PATCH] C Bindings: Allow reading MDNode operands

[Anthony Bryant]

Hi Duncan,

>>>> (Sorry for sending this to the wrong list earlier.)
>>>>
>>>> This patch just adds two functions: LLVMGetMDNodeNumOperands and
>>>> LLVMGetMDNodeOperands, which allow access to MDNode operands from the
>>>> C API, similarly to how named metadata can be accessed there.
>>>>
>>>> Could this be reviewed and possibly committed please?
>>>
>>> if the node isn't an MDNode shouldn't it barf (eg abort)?
>>
>> I thought I checked for that - both of them do dyn_cast<MDNode> and
>> check whether the result is null before continuing.
>>
>> Do you mean they should return some sort of error? Because the
>> functions that read the same things from NamedMDNodes don't.
>
> yes, that they should cause some sort of error.  I think routines like
> LLVMGetNamedMetadataNumOperands are wrong to work the way they do.  There
> should be a routine to get the named metadata pointer given the name (or
> null if there is no name).  There should be a routine to get or create
> named metadata given the name (and it should return a metadata pointer).
> Routines like LLVMGetNamedMetadataNumOperands should take the metadata
> pointer and ensure a crash if it isn't the right kind of pointer (eg by
> using cast rather than dyn_cast).

Here's an updated patch that uses cast<MDNode> instead.
I would have made your suggested changes to the named metadata
functions too, but 1) The old functions would have to stay to be
backwards compatible, and they're using the names we would probably
want to use for the new functions, and 2) NamedMDNode isn't a Value,
so I'm not sure how to go about exposing it.

Regardless, is this patch ok to go in now?

Thanks,
Anthony.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mdnode_operands_v2.patch
Type: application/octet-stream
Size: 1647 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120913/4d189597/attachment.obj>