<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jun 13, 2011, at 5:00 PM, Richard Trieu wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><br><br><div class="gmail_quote">On Thu, May 26, 2011 at 3:10 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="gmail_quote"><div class="im">On Mon, May 2, 2011 at 5:48 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br></div><div class="im">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span style="font-family:Verdana, Arial, Helvetica, sans-serif;font-size:12px"><pre style="font-size:12px"><span>Add warning when NULL constant value is used improperly in expressions.</span></pre></span></blockquote><div>
<br></div></div><div>Some high-level comments:</div><div><br></div><div>1) Why only handle GNUNull null-pointer-constants? I think there is a good reason here (making them behave as much like nullptr as possible) but it would be good to document that in comments at least.</div>
</div></blockquote><div>The other kinds of nulls are zero integer and c++0x nullptr. Integral zero is valid for these operations. nullptr is a different type which already produces an error in these cases. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="gmail_quote">
<div><br></div><div>2) What drives the set of invalid operations? I assume its those that nullptr would be invalid for? If so, can you provide standard citations against the draft standard? Also, is there no way to just turn on the C++0x errors for nullptr when we see the GNUNull in C++03, but down-grade them to warnings?</div>
</div></blockquote><div>These operations are the ones where the null pointer would be used as an integer instead of a pointer. I've also copied the test, but using nullptr instead to show that they error in same places.</div>
<div><br></div><div>It should be possible to change the NULL into a nullptr and then run the checks, but that would probably involve touching code in all the of operation checking functions. I feel that it would be better to keep this code in one place instead of spread across so many functions.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote">
<div><br></div><div>3) Because these are warnings, we can't return ExprError; that changes the parse result.</div></div>
</blockquote></div>Removed the early exit with ExprError. </blockquote><br></div><div><br></div><div><div>+ bool LeftNull = Expr::NPCK_GNUNull ==</div><div>+ lhs.get()->isNullPointerConstant(Context,</div><div>+ Expr::NPC_ValueDependentIsNotNull);</div><div>+ bool RightNull = Expr::NPCK_GNUNull ==</div><div>+ rhs.get()->isNullPointerConstant(Context,</div><div>+ Expr::NPC_ValueDependentIsNotNull);</div><div>+</div><div>+ // Detect when a NULL constant is used improperly in an expression.</div><div>+ if (LeftNull || RightNull) {</div><div><br></div><div>I think you want: </div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>if (LeftNull != RightNull) {</div><div><br></div><div>here? At the very least, we shouldn't emit two warnings when both sides of the operator are NULL.</div><div><br></div><div><div>+ // These are the operations that would not make sense with a null pointer</div><div>+ // if the other expression the other expression is not a pointer.</div><div>+ if ((LeftNull != RightNull) && !lhs.get()->getType()->isPointerType() &&</div><div>+ !rhs.get()->getType()->isPointerType()) {</div><div><br></div><div>You also need to check for member pointers, block pointers, and Objective-C pointers here.</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div></div></div></body></html>