This is an archive of the discontinued LLVM Phabricator instance.

Expose AttributeSetNode, use it to provide aggregate getter for attribute in the C API.
ClosedPublic

Authored by deadalnix on Jun 11 2016, 7:54 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

deadalnix updated this revision to Diff 60455.Jun 11 2016, 7:54 PM
deadalnix retitled this revision from to Expose AttributeSetNode, use it to provide aggregate getter for attribute in the C API..
deadalnix updated this object.
whitequark accepted this revision.Jun 11 2016, 8:51 PM
whitequark edited edge metadata.

Aggregate getters LGTM

This revision is now accepted and ready to land.Jun 11 2016, 8:51 PM
deadalnix updated this revision to Diff 60462.Jun 11 2016, 11:35 PM
deadalnix edited edge metadata.

Rebase and fix conflicts

bkramer edited edge metadata.Jun 13 2016, 12:14 PM

Exposing the guts of the attribute implementation is really ugly. Why can't this be supported through AttributeSet?

@bkramer AttributeSet simply doesn't provide the required infos here. I'm open to alternative way to expose these (that's why I wanted your feedback here). Do you have a suggestion ?

deadalnix updated this revision to Diff 60798.Jun 15 2016, 12:58 AM
deadalnix edited edge metadata.

Add CallSite support

deadalnix updated this revision to Diff 64809.Jul 20 2016, 7:22 PM

Move AttributeSetNode.h for include to lib so that it is not exposed anymore.

This revision was automatically updated to reflect the committed changes.

@whitequark , @bkramer don't wanted to expose AttributeSetNode so I moved it back to lib .

@deadalnix, So, no aggregate getters at all? That's unacceptable.

Aggregate getters are exposed. AttributeSetNode isn't, as per @bkramer 's request (it was in the initial diff).

Sorry for dropping the ball here. All I wanted is that AttributeSetNode doesn't become part of the public API, i.e. being in a header in include/llvm. Patch is fine now. I know it's annoying but I would've approved this with the latest change if you pinged me again after that.

Good to see everyone got what they wanted. Sorry for the screw up on my side, I forgot that @whitequark accepted the diff a while ago and assumed someone accepted the newer revision when I saw it was green. I apologize.