[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