[PATCH] D31198: [IR] Make AttributeSetNode public, avoid temporary AttributeList copies

Pete Cooper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 10 13:10:25 PDT 2017


pete accepted this revision.
pete added a comment.
This revision is now accepted and ready to land.

Sorry for forgetting to review this.

LGTM with just these comments (but only comments so feel free to commit).

My only worry is that I view AttributeSetNode as an internal detail of the Attribute implementation.  I think this is an incremental patch which eventually gets us to a cleaner set of APIs, so I'm fine with it for now, but I think the eventual goal shouldn't have AttributeSetNode be visible any more.

Note, when i looked at this a while ago, i created something I called AttributeRef which I guess was effectively the same as what you have here.  But it still suffered from similar issues in terms of leaking implementation details.  Its also possible that moving Attributes to the parameters just fixes most of this anyway.


https://reviews.llvm.org/D31198





More information about the llvm-commits mailing list