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

[Chandler Carruth]

On Fri, Feb 8, 2013 at 12:59 AM, David Chisnall <David.Chisnall at cl.cam.ac.uk
> wrote:

> I have tracked the last three rewrites of the Attributes APIs in some
> out-of-tree code and in none of them have I see any documentation
> describing how to migrate from the old APIs.
>

I sympathize with your desire for more documentation of the end-state APIs
David, but I think you're going too far. You can argue on mailing lists
that things be done differently, but in the LLVM project I think that Bill
is within his rights to disagree about the best approach. I don't think
that it is really productive to demand so forcefully that people change
their development model to fit your suggestions. If you firmly believe that
you're method is dramatically better, I would suggest leading by example
and demonstrate how to better refactor fundamental LLVM APIs.

I also think you should remember that it is explicitly *not* a goal of the
LLVM project to optimize its development process for out-of-tree projects,
and instead it *is* a goal to optimize the development process for in-tree
efforts. I think this is a good thing, and unlikely to change. As such, I
think staging documentation updates to follow after the APIs stabilize is
not a completely unreasonable or unacceptable approach.


Most iterations, including the current one, had nothing even approaching a
> high-level overview of the relevant classes.  And no, go read the code is
> absolutely not documentation.  Tracking down exactly which commit was
> responsible for the API change is nontrivial, and then finding all of the
> related changes and trying to map them back to your own code is not an
> adequate thing to recommend (as you have done twice now in this thread).
>  It means that every downstream user EACH has to spend more time tracking
> down what the changes are than it would take the person changing the APIs
> to just write a few quick comments in a header.
>

I'm not really disagreeing with the idea that we need documentation here...
but maybe you could help write some of the documentation since you are
impacted by these changes?


I apologise if you feel that you are being singled out here, but the code
> that interfaces with attributes is currently responsible for about 60% of
> the #ifdefs in my code that has to work with head and the last two
> releases.  For something that is effectively a bit of abstraction on a
> bitfield, this is a staggering amount of API churn.  Nothing else comes
> close.  The second largest is Chandler's refactoring of the header
> locations, and that was preceded by a discussion on the mailing list and a
> detailed design and rationale.  The attributes API changes have no public
> design document and no evidence that such documentation exists: if it did
> then writing documentation for users would have been trivial.
>

This isn't accurate. There were in fact mailing list discussions of the
ideas behind the attribute API changes. Also, several other developers
talked directly with Bill during the rewrite, and if we had significant
concerns, would have raised them using the normal post-commit review
process.

I specifically encouraged some of the abstractions that Bill has built
because the whole point, the very core idea that was discussed on the
mailing list, is that we need attributes to represent a lot more than just
a bitfield.


> If you fixed all of the in-tree consumers of these APIs, then you must be
> aware of how pervasive they are and be able to infer that they are likely
> to have a similar number of out-of-tree users.  A little courtesy to these
> people is all that I am requesting.  I realise that this is not entirely
> your fault, as the lack of documentation should have been caught in
> pre-commit review, and so the rest of us should have done a better job at
> that stage.  As such, I am willing to volunteer to review the next round of
> changes.  Perhaps you could send me the link to where the current set was
> reviewed, and let me know when you upload the next set?
>

I don't see any need for pre-commit review here. If you see an API change
go in which could use more documentation, provide that concrete feedback in
a brief and direct email replying to the commit in question. No need for a
lengthy essay or discussion.

Even better, you could help Bill by providing patches for some
documentation, as the refactoring of the attribute APIs and representation
is an important task that no one, including you, have stepped forward to
help with...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130208/ae561e3d/attachment.html>