<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Dec 10, 2013, at 4:38 , Anders Rönnholm <<a href="mailto:Anders.Ronnholm@evidente.se">Anders.Ronnholm@evidente.se</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Are you OK to commit this patch or do you see more issues?<br></blockquote></div><div><br></div><div>I'm not sure if anyone else has ideological concerns. There's always a flag to turn this off, I suppose.</div><div><br></div><div><br></div><div>+ if (S.isSFINAEContext())</div><div>+ return;</div><div><br></div><div>Code style: extra indent?</div><div><br></div><div><br></div><div><div>+ if(E->HasSideEffects(S.getASTContext()))</div><div>+ return;</div><div><br></div><div>sizeof doesn't evaluate its argument, so I'm not sure why you wouldn't want to warn here.</div><div><br></div><div><br></div><div>+ const FunctionDecl *FD = S.getCurFunctionDecl();</div><div>+ if(FD && FD->isFunctionTemplateSpecialization())</div><div>+ return;</div></div><div><br></div><div>Code style: space after if. (Above too, actually.)</div><div><br></div><div><br></div><div><div><div>+ if (Binop->getLHS()->getType()->isArrayType() ||</div><div>+ Binop->getLHS()->getType()->isAnyPointerType() ||</div><div>+ Binop->getRHS()->getType()->isArrayType() ||</div><div>+ Binop->getRHS()->getType()->isAnyPointerType())</div><div>+ return;</div><div><br></div><div>I don't think this is correct...the user is only trying to get ptrdiff_t if <i>both</i> the LHS and RHS are pointer-ish.</div><div><br></div></div></div><div><br></div><div><div>+def warn_sizeof_bin_op : Warning<</div><div>+ "using sizeof() on an expression with an operator may yield unexpected results">,</div><div>+ InGroup<SizeofOnExpression>;</div><div>+</div><div>+def warn_sizeof_sizeof : Warning<</div><div>+ "using sizeof() on sizeof() may yield unexpected results.">,</div><div>+ InGroup<SizeofOnExpression>;</div><div>+ </div></div><div><br></div><div>sizeof doesn't actually require parens, so we shouldn't put the parens in the diagnostics.</div></body></html>