On Sun, Jan 22, 2012 at 9:48 AM, Eli Friedman <[email protected]> wrote: > On Sun, Jan 22, 2012 at 12:31 AM, Aaron Ballman <[email protected]> > wrote: >> On Sun, Jan 22, 2012 at 8:58 AM, Eli Friedman <[email protected]> wrote: >>> On Sat, Jan 21, 2012 at 11:38 PM, Aaron Ballman <[email protected]> >>> wrote: >>>> On Thu, Jan 19, 2012 at 12:00 AM, Eli Friedman <[email protected]> >>>> wrote: >>>>> On Mon, Jan 16, 2012 at 8:38 PM, Aaron Ballman <[email protected]> >>>>> wrote: >>>>>> On Sun, Jan 15, 2012 at 9:05 PM, Eli Friedman <[email protected]> >>>>>> wrote: >>>>>>> On Sun, Jan 15, 2012 at 1:45 PM, Aaron Ballman <[email protected]> >>>>>>> wrote: >>>>>>>> This is a patch for fixing 11747, allowing reinterpret_cast to "cast" >>>>>>>> where the source and destination types are the same as per >>>>>>>> [expr.reinterpret.cast]p2. The fix itself is fairly simple -- the >>>>>>>> code was already there, it just needed to be moved up slightly since >>>>>>>> the previous check was for source and dest being pointers. >>>>>>> >>>>>>> C++11 [expr.reinterpret.cast]p2 is as follows: >>>>>>> >>>>>>> The reinterpret_cast operator shall not cast away constness (5.2.11). >>>>>>> An expression of integral, enumeration, pointer, or pointer-to-member >>>>>>> type can be explicitly converted to its own type; such a cast yields >>>>>>> the value of its operand. >>>>>> >>>>>> Attached is a new patch to hopefully better address this, as well as >>>>>> some additional test cases. >>>>> >>>>> + // C++ 5.2.10p2 has a note that mentions that, subject to all other >>>>> + // restrictions, a cast to the same type is allowed so long as it >>>>> does not >>>>> + // cast away constness. The intent is not entirely clear here, since >>>>> all >>>>> + // other paragraphs explicitly forbid casts to the same type. >>>>> + // >>>>> + // The only allowed types are: integral, enumeration, pointer, or >>>>> + // pointer-to-member types. >>>>> >>>>> This comment is really unclear... you should really explicitly state >>>>> that the C++98 version is unclear, and C++11 clarifies this case. >>>> >>>> I've clarified now, thanks for pointing out the confusion! >>>> >>>>> + } else if (SrcType->isPointerType() || >>>>> + SrcType->isMemberPointerType()) { >>>>> >>>>> The original code allowed "DestType->isAnyPointerType() || >>>>> DestType->isBlockPointerType()". I don't see any reason to restrict >>>>> Objective-C or block pointers. >>>> >>>> Nor do I, I've added this in as well. >>>> >>>>> + if (CastsAwayConstness(Self, SrcType, DestType, >>>>> /*CheckCVR=*/!CStyle, >>>>> + /*CheckObjCLifetime=*/CStyle)) { >>>>> >>>>> How could a cast where the source and destination types are identical >>>>> cast away constness? >>>> >>>> From my testing, the equality test does not take cv qualifiers into >>>> account. Is there another way I should be testing for equality that >>>> does? >>> >>> "==" should definitely be taking qualifiers into account. Do you have >>> a testcase? >> >> It was in the patch -- but you can reproduce it yourself pretty easily: >> >> void foo() { >> const int i = 0; >> (void)reinterpret_cast< int >( i ); >> } >> >> Put a breakpoint in SemaCast.cpp on or around line 1629 after applying >> the patch and you'll see that you break into the SrcType == DestType >> block. > > Isn't that just lvalue-to-rvalue conversion stripping off qualifiers?
I think you're right -- as soon as I use pointers, the equality test fails as you predicted. I've adjusted my patch accordingly, thanks for the thorough review! ~Aaron
reinterpret_cast.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
