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

Hi,

I recently upgraded to the latest LLVM build and encountered a problem where the
API for Argument::addAttr has changed.

Previously it was Argument::addAttr(Attribute A) and I was able to work with
this.

The latest build has changed the method addAttr so that it requires an
AttributeSet argument (Argument::addAttr(AttributeSet AS).

I'm not sure how to adjust to this change. The AttributeSet object seems to
store an array of Attribute(s) to construct an AttributeSet argument for addAttr
I need to know the index of the Argument in the function?

I can follow the lead of this code from Core.cpp:

void LLVMAddAttribute(LLVMValueRef Arg, LLVMAttribute PA) {
  Argument *A = unwrap<Argument>(Arg);
  AttrBuilder B(PA);
  A->addAttr(AttributeSet::get(A->getContext(), A->getArgNo() + 1, 
B));
}


Is this all I need?  What does the A->getArgNo()+1 provide? The index of the
argument in the function argument list+1?

Is this change to the API described anywhere?

Best,

.Chris.


-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130130/bc202cf3/attachment.html>

On Jan 30, 2013, at 8:20 PM, Christian Schafmeister <chris.schaf at
verizon.net> wrote:
> 
> Hi,
> 
> I recently upgraded to the latest LLVM build and encountered a problem
where the API for Argument::addAttr has changed.
> 
> Previously it was Argument::addAttr(Attribute A) and I was able to work
with this.
> 
> The latest build has changed the method addAttr so that it requires an
AttributeSet argument (Argument::addAttr(AttributeSet AS).
> 
Yes.
> I'm not sure how to adjust to this change. The AttributeSet object
seems to store an array of Attribute(s) to construct an AttributeSet argument
for addAttr I need to know the index of the Argument in the function?
> 
That's correct.
> I can follow the lead of this code from Core.cpp:
> 
> void LLVMAddAttribute(LLVMValueRef Arg, LLVMAttribute PA) {
>   Argument *A = unwrap<Argument>(Arg);
>   AttrBuilder B(PA);
>   A->addAttr(AttributeSet::get(A->getContext(), A->getArgNo() + 1,
B));
> }
> 
> 
> Is this all I need?  What does the A->getArgNo()+1 provide? The index of
the argument in the function argument list+1?
> 
Yes. :-) It's the index of the argument into the list. The indices go like
this:

Index number
------------
     0       <- The attribute for the return
    ~0U      <- The index for the function
1, 2, ..., n <- The indexes for the arguments to the function

The argument number that you get from "A->getArgNo()" is 0-based,
so you need the "+ 1" to get the correct index into the AttributeSet.
> Is this change to the API described anywhere?
> 
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).

-bw

> However, we do list the thing we do change in the ReleaseNotes
Unfortunately this doesn't actually seem to be the case.

-- Sean Silva

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.

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.

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

David