A few thoughts:
Could this be generalized to fit _Alignof and _Generic (the
controlling expression) as well? The behavior is the same for all
three (they're unevaluated).
Also, in terms of sizeof, if the expression modifies a VLA, the
behavior is sadly well-defined (in C). Eg)
size_t f(size_t n) {
/* n must be incremented */
size_t a = sizeof(int[++n]);
return n;
}
This will return n + 1 because it affects the size of the VLA. So we
probably should not warn on that case.
~Aaron
On Thu, Jan 23, 2014 at 9:40 AM, Anders Rönnholm
<[email protected]> wrote:
> Hi,
>
> New one with comments handled.
>
> ________________________________________
> Från: Jordan Rose [[email protected]]
> Skickat: den 20 december 2013 19:15
> Till: Anders Rönnholm
> Cc: [email protected]; Daniel Marjamäki; Anna Zaks; David Blaikie;
> Richard Smith; Matt Calabrese
> Ämne: Re: [PATCH] [StaticAnalyzer] New checker Sizeof on expression
>
> On Dec 10, 2013, at 4:38 , Anders Rönnholm
> <[email protected]<mailto:[email protected]>> 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.
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
