nikic added a reviewer: aeubanks.
nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

This looks good to me, but I'd recommend waiting a bit before landing in case 
there's further input, as this is a non-trivial API change.

I think separating AttrBuilder (which stores attribute keys and values) from 
AttributeMask (which only stores keys) makes sense conceptually. With the 
current API, it's not really obvious that the attribute values are completely 
ignored for removal operations -- on the surface it looks like the attribute 
would only get removed if both key and value match, but that's not how it 
actually works.

This should also give more flexibility in optimizing AttrBuilder for it's 
primary use case (which is adding attributes).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116110/new/

https://reviews.llvm.org/D116110

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to