[LLVMdev] Question about changes to llvm::Argument::addAttr(AttributeSet AS) API

[Bill Wendling]

On Feb 4, 2013, at 11:54 PM, David Chisnall <David.Chisnall at cl.cam.ac.uk> wrote:

> On 5 Feb 2013, at 01:32, Bill Wendling wrote:
> 
>> No. It hasn't been written up. We typically don't do write-ups for API changes. However, we do list the thing we do change in the ReleaseNotes (these changes haven't made it there though).
> 
> The attributes API has undergone a horrendous amount of churn over the last few months, both before and after the 3.2 release.  I've lost track of the number of times I've rewritten code interacting with this API over the past year and I now have a mess of #ifdefs just to support 3.1, 3.2 and trunk.  Each time, the only warning I got was that my code stopped building with trunk, and the only way of knowing how to rewrite it was to read the attributes code.
> 
I'm sorry about the problems. It's a continuing problem when working with a project where the APIs are not set in stone and are subject to change at a moment's notice.

> It would be really great if whoever is responsible could step back from their code editor for a few minutes and write some documentation on how to migrate from each iteration to the next.
> 
That would be more trouble than it's worth because the API is changing quite rapidly right now. A lot of it hasn't solidified just yet. Basically, once a migration doc was written it would be immediately out of date.

> One of the goals of LLVM is to be a set of reusable libraries and this goal is not met by gratuitous API churn with no accompanying documentation.  If we can't have a sane deprecation strategy or an automated migration tool then please can we at least have a token attempt at documentation?  The AttributeSet documentation is just embarrassing.  For example, the document for the class is:
> 
>> This class manages the ref count for the opaque AttributeSetImpl
>> object and provides accessors for it
> 
> Great.  It's a wrapper around an opaque type that isn't documented in the public headers.  What is an AttributeSetImpl?  What do I use it for?  How does it relate to the last three classes that had similar functionality but different names?

It's an opaque object. It shouldn't concern people using the AttributeSet class. It's not documented in public headers for that very reason.

> I can tell it uses the pImpl pattern by just looking at the class definition.  That is not what the documentation should be telling me.  The rest of this file is a case study in how not to write helpful documentation.  This wouldn't be quite so bad if it were a class that's fairly obscure, but this is a class that is part of the core IR that pretty much every part of the pipeline needs to interact with.  (And that's ignoring the fact that it is very confusing for an ordered collection that allows duplicates to be called a set).
> 
Welcome to living on the top-of-tree! :-)

We are in between releases. It's expected that the APIs will be unstable. I wrote (several times) that the attributes were going to be rewritten. That implies a lot of things, including that everything about attributes and how you use them will change. Where applicable, there will be an auto-upgrade done. (One requirement is that the old bitcode files will continue to be parsed correctly (at least until release 4.0).) And yes the documentation is also in flux. It has been improving over time. The thing you quoted above is perhaps the worst comment from the Attributes.h file, and isn't indicative of the rest of the comments in there, which have been changing during the rewrite.

-bw

P.S. AttributeSets don't allow duplicates. :)