[PATCH] D15844: [ADT] Add an abstraction for embedding an integer within a pointer-like type.

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

In http://reviews.llvm.org/D15844#322318, @jordan_rose wrote:

> Ha, I wrote the same thing, but it's only used for Swift-Clang's "API notes" feature that hasn't been upstreamed, so it's still sitting in our repository. I called it Fixnum <https://github.com/apple/swift-llvm/blob/upstream-with-swift/include/llvm/ADT/Fixnum.h>. Sorry about that.


No problem, seems like this is going in the right direction anyways. =]

> Differences:

> 

> - when stored outside a PointerIntPair, it uses the smallest storage possible

> - safe upcasting is implicitly allowed

> - assignment/construction between bit widths is permitted but checked

> - I hadn't written special DenseMapInfo (because we never used it in a DenseMap)

> 

>   I'm fine switching ours over to yours, or you taking ours as is. I really thought we had tests for it too but I can't find them.


Well, the approach I've taken (storing it as a pointer, and quickly converting it an integer type for any/all operations) is at least from the code size much simpler, so I have a bias towards that.

Let's start with the patch as I have it since it at least works for some use cases and is really boring and simple. It also has basic tests that can be extended incrementally.

So far at least, I have found it really effective to rely on the single implicit conversion to a particular integer type, and then allow promotion and normal arithmetic to take place. This seemed to work well. The typical result I expect is that the ADT type only really exists inside of a sum type or pointer union. All the other places, you just get the integer.

However, I've not used it super widely, so if you try to use this in Swift you may uncover reasons why we need to add some (or all) of the facilities from your version. If/when you do, you can post the patches to merge functionality into this (along with tests that show what usage pattern needs it) until it is sufficient for the use cases you have in Swift.

I'm going to land this as is since you seem fine with it, but just follow up with any issues you run into when it shows up on the swift side of things. Thanks!

-Chandler


http://reviews.llvm.org/D15844