<div class="gmail_quote">I share John and Doug's concern over false positive rates. However, I'd like to evaluate that by getting a conservative version of this in, trying it on some code, and evaluating the fallout. If this isn't useful we can drop it.</div>
<div class="gmail_quote"><br></div><div class="gmail_quote">However, I think we could do a couple of things more to avoid false-positives:</div><div class="gmail_quote"><br></div><div class="gmail_quote">1) Only warn on reference reinterpret casts when the result is in an lvalue-to-rvalue cast to avoid this false positive:</div>
<div class="gmail_quote"><br></div><div class="gmail_quote">float f = 0.0f;</div><div class="gmail_quote">int *x = &reinterpret_cast<int&>(f);</div><div class="gmail_quote"><br></div><div class="gmail_quote">
2) Don't warn for reinterpret casts of tag types. The original bug this was intended to catch were people abusing reinterpret cast to get at the bits of a float. We can catch that without flagging type-system manipulations and struct munging that are fairly common. This will allow us to catch the obvious bugs surround primitive types, function pointers, etc.</div>
<div class="gmail_quote"><br></div><div class="gmail_quote">John, Doug, would that alleviate some of your concerns?</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Tue, Mar 29, 2011 at 1:11 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Revising the patch per your suggestions:<div>1) Make this warning off by default</div><div>2) Removing unrelated whitespace changes</div>
<div>3) Replace underscores with hyphens in diagnostic group name<div><div></div><div class="h5"><br><br><div class="gmail_quote">
On Sun, Mar 27, 2011 at 3:13 AM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>On Mar 27, 2011, at 2:44 AM, Douglas Gregor wrote:<br>
> On Mar 22, 2011, at 10:24 PM, Richard Trieu wrote:<br>
>> Add a warning for when reinterpret_cast leads to undefined behavior. Also updated reinterpret_cast tests and turned off this warning for tests that don't need it.<br>
><br>
> I'm not totally convinced that this warning is useful. The issue is that we're not really invoking undefined behavior by performing the cast, or even dereferencing the result of that cast, so long as the memory we're pointing to actually has an object of the right type. However, that's not what this warning is checking: this warning is checking whether the declared type before the cast, and the type we're casting to, are compatible.<br>
><br>
> Overall, I expect that this warning will have a fairly high false-positive rate, since programmers tend to use reinterpret_cast in very specific ways where they know far more about the type of the object behind the pointer/reference than the type system says. To really catch problems with reinterpret_cast, we're likely to need some stronger analysis (e.g., something in the static analyzer) that checks that we aren't referring to the same lvalue in different places with different, incompatible types.<br>
<br>
</div>This isn't just an abstract point — we use reinterpret_cast in various Clang headers to convert up and down class hierarchies that we don't want to require the inclusion of headers for. In addition, while reinterpreting a pointer to a double object as an int* and accessing it is technically undefined behavior because of aliasing rules, it's actually such an important idiom that a major design goal of the LLVM aliasing optimization has been to make sure that "sufficiently obvious" code is not broken by strict aliasing.<br>
<br>
I think that even if this warning were doing the more sophisticated analysis Doug has in mind, it would still be inappropriate for it to be enabled by default.<br>
<br>
Also, your patch has unrelated whitespace changes; please remove these.<br>
<font color="#888888"><br>
John.</font></blockquote></div><br></div></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br>