On Sun, Jan 22, 2012 at 12:56 AM, Aaron Ballman <[email protected]> wrote: > 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!
+ // 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. In C++98 The intent is not entirely clear here, + // since all other paragraphs explicitly forbid casts to the same type. + // In C++11 clarifies this case. It looks like this comment somehow got messed up in editing. Otherwise, looks good. -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
