<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">LGTM<br><div><div>On Apr 6, 2013, at 8:53 AM, Joe Groff <<a href="mailto:arcata@gmail.com">arcata@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">On Fri, Apr 5, 2013 at 9:03 AM, Evan Cheng <span dir="ltr"><<a href="mailto:evan.cheng@apple.com" target="_blank">evan.cheng@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Joe,<br>
<br>
Using "Int" as variable name seems like a bad idea to me. Could you fix it while you are fixing the class?<br></blockquote><div><br></div><div style="">Sure. How does this look?</div><div style=""><br></div><div style="">
-Joe</div><div style=""><br></div><div>Index: include/llvm/ADT/PointerIntPair.h</div><div>===================================================================</div><div>--- include/llvm/ADT/PointerIntPair.h<span class="" style="white-space:pre"> </span>(revision 178549)</div>
<div>+++ include/llvm/ADT/PointerIntPair.h<span class="" style="white-space:pre"> </span>(working copy)</div><div>@@ -29,7 +29,7 @@</div><div> /// on the number of bits available according to PointerLikeTypeTraits for the</div>
<div> /// type.</div><div> ///</div><div>-/// Note that PointerIntPair always puts the Int part in the highest bits</div><div>+/// Note that PointerIntPair always puts the IntVal part in the highest bits</div><div> /// possible. For example, PointerIntPair<void*, 1, bool> will put the bit for</div>
<div> /// the bool into bit #2, not bit #0, which allows the low two bits to be used</div><div> /// for something else. For example, this allows:</div><div>@@ -57,13 +57,13 @@</div><div> };</div><div> public:</div><div>
PointerIntPair() : Value(0) {}</div><div>- PointerIntPair(PointerTy Ptr, IntType Int) {</div><div>+ PointerIntPair(PointerTy PtrVal, IntType IntVal) {</div><div> assert(IntBits <= PtrTraits::NumLowBitsAvailable &&</div>
<div> "PointerIntPair formed with integer size too large for pointer");</div><div>- setPointerAndInt(Ptr, Int);</div><div>+ setPointerAndInt(PtrVal, IntVal);</div><div> }</div><div>- explicit PointerIntPair(PointerTy Ptr) {</div>
<div>- initWithPointer(Ptr);</div><div>+ explicit PointerIntPair(PointerTy PtrVal) {</div><div>+ initWithPointer(PtrVal);</div><div> }</div><div> </div><div> PointerTy getPointer() const {</div><div>@@ -75,41 +75,41 @@</div>
<div> return (IntType)((Value >> IntShift) & IntMask);</div><div> }</div><div> </div><div>- void setPointer(PointerTy Ptr) {</div><div>- intptr_t PtrVal</div><div>- = reinterpret_cast<intptr_t>(PtrTraits::getAsVoidPointer(Ptr));</div>
<div>- assert((PtrVal & ((1 << PtrTraits::NumLowBitsAvailable)-1)) == 0 &&</div><div>+ void setPointer(PointerTy PtrVal) {</div><div>+ intptr_t PtrWord</div><div>+ = reinterpret_cast<intptr_t>(PtrTraits::getAsVoidPointer(PtrVal));</div>
<div>+ assert((PtrWord & ((1 << PtrTraits::NumLowBitsAvailable)-1)) == 0 &&</div><div> "Pointer is not sufficiently aligned");</div><div> // Preserve all low bits, just update the pointer.</div>
<div>- Value = PtrVal | (Value & ~PointerBitMask);</div><div>+ Value = PtrWord | (Value & ~PointerBitMask);</div><div> }</div><div> </div><div>- void setInt(IntType Int) {</div><div>- intptr_t IntVal = Int;</div>
<div>- assert(IntVal < (1 << IntBits) && "Integer too large for field");</div><div>+ void setInt(IntType IntVal) {</div><div>+ intptr_t IntWord = static_cast<intptr_t>(IntVal);</div>
<div>+ assert(IntWord < (1 << IntBits) && "Integer too large for field");</div><div> </div><div> // Preserve all bits other than the ones we are updating.</div><div> Value &= ~ShiftedIntMask; // Remove integer field.</div>
<div>- Value |= IntVal << IntShift; // Set new integer.</div><div>+ Value |= IntWord << IntShift; // Set new integer.</div><div> }</div><div> </div><div>- void initWithPointer(PointerTy Ptr) {</div>
<div>- intptr_t PtrVal</div><div>- = reinterpret_cast<intptr_t>(PtrTraits::getAsVoidPointer(Ptr));</div><div>- assert((PtrVal & ((1 << PtrTraits::NumLowBitsAvailable)-1)) == 0 &&</div><div>
+ void initWithPointer(PointerTy PtrVal) {</div><div>+ intptr_t PtrWord</div><div>+ = reinterpret_cast<intptr_t>(PtrTraits::getAsVoidPointer(PtrVal));</div><div>+ assert((PtrWord & ((1 << PtrTraits::NumLowBitsAvailable)-1)) == 0 &&</div>
<div> "Pointer is not sufficiently aligned");</div><div>- Value = PtrVal;</div><div>+ Value = PtrWord;</div><div> }</div><div> </div><div>- void setPointerAndInt(PointerTy Ptr, IntType Int) {</div>
<div>- intptr_t PtrVal</div><div>- = reinterpret_cast<intptr_t>(PtrTraits::getAsVoidPointer(Ptr));</div><div>- assert((PtrVal & ((1 << PtrTraits::NumLowBitsAvailable)-1)) == 0 &&</div><div>
+ void setPointerAndInt(PointerTy PtrVal, IntType IntVal) {</div><div>+ intptr_t PtrWord</div><div>+ = reinterpret_cast<intptr_t>(PtrTraits::getAsVoidPointer(PtrVal));</div><div>+ assert((PtrWord & ((1 << PtrTraits::NumLowBitsAvailable)-1)) == 0 &&</div>
<div> "Pointer is not sufficiently aligned");</div><div>- intptr_t IntVal = Int;</div><div>- assert(IntVal < (1 << IntBits) && "Integer too large for field");</div><div>
+ intptr_t IntWord = static_cast<intptr_t>(IntVal);</div><div>+ assert(IntWord < (1 << IntBits) && "Integer too large for field");</div><div> </div><div>- Value = PtrVal | (IntVal << IntShift);</div>
<div>+ Value = PtrWord | (IntWord << IntShift);</div><div> }</div><div> </div><div> PointerTy const *getAddrOfPointer() const {</div><div style=""> </div></div></div></div>
</blockquote></div><br></body></html>