[PATCH] D32819: [IR] Switch AttributeList to use an array for O(1) access

[Tobias Edler von Koch via Phabricator via llvm-commits]

tobiasvk added a comment.

In https://reviews.llvm.org/D32819#749163, @rnk wrote:

> One question remains: do you think I should completely encapsulate this indexing change by adding 1 inside AttributeList::getAttributes, or is it better to update all the callers as I did in this change? The problem with this change is that it will break out-of-tree backends and frontends that use the APIs with one-based indexing. It won't even be a compile failure because the APIs still take `unsigned` integers. That's a bit rude without some more hand waving on llvm-dev and release notes.


I'm very concerned about breaking out-of-tree code in a way that's not detectable at compile time. We have a whole bunch of out-of-tree passes that will be affected by this (...I'll fix them since I know about it) but others may not be watching the mailing lists as closely. Of course we make no guarantees about C++ API stability, but I feel we shouldn't cause unnecessary pain and breakage if it can be avoided.

How about renaming the APIs that take or return arg indices? That'll make it abundantly clear that their behavior has changed (and will also easily flag up all the places in upstream code that need to be changed).


https://reviews.llvm.org/D32819