[PATCH 3/3] Clean up multiple minor code issues with attributes implementation

[Stephen Lin]

As suggested by Bill Wendling, I split the patch into two: one for the
first two issues and another for the naming inconsistency issue.

Please kindly review.

Thank you,
Stephen

On Mon, Apr 8, 2013 at 2:25 PM, Stephen Lin <swlin at post.harvard.edu> wrote:
> Hi,
>
> I noticed the following code issues, which are addressed by this patch:
>
> 1. Verify::VerifyParameterAttrs in "lib/IR/Verifier.cpp" and
> AttrBuilder::removeFunctionOnlyAttrs in "lib/IR/Attributes.cpp" (only
> called by Verify::VerifyFunctionAttrs) separately maintained a list of
> function-only attribute types. I've consolidated the logic into a new
> function used for both cases in "lib/IR/Verifier.cpp", so this logic
> is in one place (other than the AsmParser front-end)
>
> 2. Various functions in "lib/IR/Verifier.cpp" passed AttributeSet
> around by reference needlessly, as it's just a handle to an immutable
> pimpl body.
>
> 3. Semantics of parameters named Index and Idx were inconsistent
> between "include/llvm/IR/Attributes.h", "lib/IR/AttributeImpl.h" and
> "lib/IR/Attributes.cpp": sometimes these were fixed 1-based indexes of
> IR parameters (or AttributeSet::ReturnIndex for IR return values or
> AttributeSet::FunctionIndex for IR functions), other times they were
> the internal slot for storage in the underlying AttributeSetImpl. I
> renamed usage of the former to "Index" and usage of the latter to
> "Slot" ("Slot" was already being used consistently for the latter in a
> subset of cases)
>
> All tests in "llvm/tests" pass with these changes.
>
> Also, I noticed the following, but did not address it since I wasn't
> sure if it was intentional:
>
> 4. The parameter and return values that semantically correspond to IR
> parameter indices (rather than slots) are sometimes unsigned and
> sometimes uint64_t, even though the underlying values are stored as
> unsigned. Since some of these functions are explicitly only present
> for backward compatibility to code relying on the mask-based
> implementation of attributes, I've left them untouched just in case.
>
> Please kindly review this patch and let me know if it can be
> committed. (I don't have commit access, so if you could also do that
> on my behalf, that would be great!)
>
> Stephen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clean-verifier.patch
Type: application/octet-stream
Size: 12768 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130418/cb5dc705/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: disambiguate-idx.patch
Type: application/octet-stream
Size: 16626 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130418/cb5dc705/attachment-0001.obj>