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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In https://reviews.llvm.org/D31198#722892, @pete wrote:

> Sorry for forgetting to review this.


NP, I got distracted by PDB stuff for a bit anyway.

> 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.

I agree. I plan to wrap it back up in a new type called `AttributeSet` that will hold the enum attributes locally, so we can do much faster `hasAttr` tests.


https://reviews.llvm.org/D31198