[PATCH] [StaticAnalyzer] New checker Sizeof on expression

Jordan Rose jordan_rose at apple.com
Fri Dec 20 10:15:53 PST 2013


On Dec 10, 2013, at 4:38 , Anders Rönnholm <Anders.Ronnholm at evidente.se> wrote:

> Are you OK to commit this patch or do you see more issues?


I'm not sure if anyone else has ideological concerns. There's always a flag to turn this off, I suppose.


+  if (S.isSFINAEContext())
+      return;

Code style: extra indent?


+  if(E->HasSideEffects(S.getASTContext()))
+    return;

sizeof doesn't evaluate its argument, so I'm not sure why you wouldn't want to warn here.


+  const FunctionDecl *FD = S.getCurFunctionDecl();
+  if(FD && FD->isFunctionTemplateSpecialization())
+    return;

Code style: space after if. (Above too, actually.)


+    if (Binop->getLHS()->getType()->isArrayType() ||
+        Binop->getLHS()->getType()->isAnyPointerType() ||
+        Binop->getRHS()->getType()->isArrayType() ||
+        Binop->getRHS()->getType()->isAnyPointerType())
+      return;

I don't think this is correct...the user is only trying to get ptrdiff_t if both the LHS and RHS are pointer-ish.


+def warn_sizeof_bin_op : Warning<
+  "using sizeof() on an expression with an operator may yield unexpected results">,
+  InGroup<SizeofOnExpression>;
+
+def warn_sizeof_sizeof : Warning<
+  "using sizeof() on sizeof() may yield unexpected results.">,
+  InGroup<SizeofOnExpression>;
+  

sizeof doesn't actually require parens, so we shouldn't put the parens in the diagnostics.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131220/4b46840b/attachment.html>


More information about the cfe-commits mailing list