[cfe-commits] [PATCH] PR13824 warn if reinterpret_cast used for up/downcast

[Alexander Zinenko]

Ping?


On 28 January 2013 23:03, Alexander Zinenko <ftynse at gmail.com> wrote:

> Hello!
>
> If this patch is good enough, could someone commit it, please?
>
>
> On 24 January 2013 21:55, Alexander Zinenko <ftynse at gmail.com> wrote:
>
>> On 24 January 2013 04:33, Jordan Rose <jordan_rose at apple.com> wrote:
>>
>>>
>>> On Jan 23, 2013, at 14:53 , Dmitri Gribenko <gribozavr at gmail.com> wrote:
>>>
>>> > On Thu, Jan 24, 2013 at 12:40 AM, Alexander Zinenko <ftynse at gmail.com>
>>> wrote:
>>> >> On 23 January 2013 23:52, Dmitri Gribenko <gribozavr at gmail.com>
>>> wrote:
>>> >>> I think it is sensible to leave the warning on by default.  The
>>> >>> purpose of a separate flag is to turn off the (possibly) noisy case
>>> >>> separately.
>>> >>
>>> >> Having it on by default, clang fails to pass these tests:
>>> >>    Clang :: Analysis/inlining/dyn-dispatch-bifurcate.cpp
>>> >>    Clang :: SemaCXX/address-space-conversion.cpp
>>> >> That's why the noisy version was disabled by default even in the first
>>> >> version.
>>> >
>>> > Well, compiler tests do weird things, so this is not representative.
>>>
>>> dyn-dispatch-bifurcate is actually what encouraged me to file that PR in
>>> the first place. You can go ahead and add it as an expected warning there.
>>
>>
>>> For address-space-conversion, you could disable the warning in the RUN
>>> line.
>>>
>>
>> Thanks, I was not sure changing anyone else's test is OK.  Updated patch
>> modifies these tests.
>>
>> On Jan 23, 2013, at 14:53 , Dmitri Gribenko <gribozavr at gmail.com> wrote:
>>
>>> > I think the only reasonable false positive source for zero adjustment
>>> > case is templates:
>>> >
>>> > template<typename T, typename U>
>>> > void foo(T *t) {
>>> >  ... reinterpret_cast<U*>(t);
>>> > }
>>> >
>>> > Where T and U have some subtyping relationship.
>>>
>>
>> Warning could be disabled for dependent types.
>> Trivial case warning adds an extra note where the template was
>> instantiated, though. As for me it's worth still having it enabled.
>>
>> template.cpp:15:10: warning: 'reinterpret_cast' is used as an upcast from
>> type 'B *' to its base type 'A *' [-Wreinterpret-updown-zero-adjustment]
>>   (void)*reinterpret_cast<U *>(t);
>>          ^~~~~~~~~~~~~~~~~~~~~~~~
>> template.cpp:20:3: note: in instantiation of function template
>> specialization 'foo<B, A>' requested here
>>   foo<B, A>(&b);
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130211/faa94aba/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 13824.patch
Type: application/octet-stream
Size: 28368 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130211/faa94aba/attachment.obj>