This should be a warning, not a static analyser check, shouldn't it? On 1 Oct 2013 09:34, "Jordan Rose" <[email protected]> wrote:
> The categories are supposed to be very broad groupings: "Logic error", > "Memory leak", "Convention X". "Logic error" is probably fine here, even > though it's generic, since there's not really a group that fits this > general kind of mistake. I just want to avoid magic strings—we shouldn't > have variants of the same thing because they'll show up as different > categories. > > Comments on the patch: > > + // Only check add operators. > + if (B->getOpcode() != BO_Add) > > How about +=? > > > + const CharacterLiteral *CharExpr = > + dyn_cast<CharacterLiteral>(B->getRHS()->IgnoreImpCasts()); > > Should this be a character literal, or just any expression with "char" > type? I realize that someone could be using a char-sized variable as an > index, but that seems fairly rare, and harder to catch by hand. > > > + if (!(StringRefExpr->getType() == > + > Context.getPointerType(Context.getConstType(Context.CharTy)) || > + StringRefExpr->getType() == > Context.getPointerType(Context.CharTy))) > > Things to consider: > 1. Do explicitly signed or unsigned chars count? In that case, you should > use Type::isCharType(). > 2. Do wide chars count? Then you should use Type::isAnyCharacterType(). > 3. Even if none of the above, you should consider typedefs, so it's still > worth going through getCanonicalType() and getPointeeType() rather than > building up a new pointer type. > > One possibility: > StringRefExpr->getType()->getPointeeType()->isAnyCharacterType(). At this > point, though, StringRefExpr->getType() probably deserves a helper variable. > > > +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.core.StringPlusChar > -verify %s > > Please include the "core" package in these checks; running without core > enabled isn't a use case we're interested in. > > Jordan > > > On Sep 30, 2013, at 3:25 , Anders Rönnholm <[email protected]> > wrote: > > Hi Jordan, > > Thank you for your comments. I have changed the check to use > RecursiveASTVisitor. However i'm not sure what i should write as category > in bugreport. You mentioned adding a new category in CommonBugCategories > for SIzeofOnExpression, should i do that or is what i have enough for now? > > > I have provided a new diff. > > //Anders > > > > _______________________________________________ > 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
