Clean up the sentinel-attribute checking code a lot. Document

what 'nullPos' is supposed to mean, at least at this one site.
Use closed forms for the arithmetic.  Rip out some clever but
ultimately pointless code that was trying to use 0 or 0L depending
the size of a pointer vs. the size of int;  first, it didn't work
on LLP64 systems, and second, the sentinel checking code requires
a pointer-typed value anyway, so this fixit would not have actually
removed the warning.

llvm-svn: 139361
This commit is contained in:
John McCall 2011-09-09 07:56:05 +00:00
parent ba60b04148
commit b46f287e48
2 changed files with 69 additions and 84 deletions

View File

@ -1771,7 +1771,7 @@ static void handleSentinelAttr(Sema &S, Decl *D, const AttributeList &Attr) {
return; return;
} }
int sentinel = 0; unsigned sentinel = 0;
if (Attr.getNumArgs() > 0) { if (Attr.getNumArgs() > 0) {
Expr *E = Attr.getArg(0); Expr *E = Attr.getArg(0);
llvm::APSInt Idx(32); llvm::APSInt Idx(32);
@ -1781,16 +1781,17 @@ static void handleSentinelAttr(Sema &S, Decl *D, const AttributeList &Attr) {
<< "sentinel" << 1 << E->getSourceRange(); << "sentinel" << 1 << E->getSourceRange();
return; return;
} }
sentinel = Idx.getZExtValue();
if (sentinel < 0) { if (Idx.isSigned() && Idx.isNegative()) {
S.Diag(Attr.getLoc(), diag::err_attribute_sentinel_less_than_zero) S.Diag(Attr.getLoc(), diag::err_attribute_sentinel_less_than_zero)
<< E->getSourceRange(); << E->getSourceRange();
return; return;
} }
sentinel = Idx.getZExtValue();
} }
int nullPos = 0; unsigned nullPos = 0;
if (Attr.getNumArgs() > 1) { if (Attr.getNumArgs() > 1) {
Expr *E = Attr.getArg(1); Expr *E = Attr.getArg(1);
llvm::APSInt Idx(32); llvm::APSInt Idx(32);
@ -1802,7 +1803,7 @@ static void handleSentinelAttr(Sema &S, Decl *D, const AttributeList &Attr) {
} }
nullPos = Idx.getZExtValue(); nullPos = Idx.getZExtValue();
if (nullPos > 1 || nullPos < 0) { if ((Idx.isSigned() && Idx.isNegative()) || nullPos > 1) {
// FIXME: This error message could be improved, it would be nice // FIXME: This error message could be improved, it would be nice
// to say what the bounds actually are. // to say what the bounds actually are.
S.Diag(Attr.getLoc(), diag::err_attribute_sentinel_not_zero_or_one) S.Diag(Attr.getLoc(), diag::err_attribute_sentinel_not_zero_or_one)
@ -1812,9 +1813,7 @@ static void handleSentinelAttr(Sema &S, Decl *D, const AttributeList &Attr) {
} }
if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) { if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
const FunctionType *FT = FD->getType()->getAs<FunctionType>(); const FunctionType *FT = FD->getType()->castAs<FunctionType>();
assert(FT && "FunctionDecl has non-function type?");
if (isa<FunctionNoProtoType>(FT)) { if (isa<FunctionNoProtoType>(FT)) {
S.Diag(Attr.getLoc(), diag::warn_attribute_sentinel_named_arguments); S.Diag(Attr.getLoc(), diag::warn_attribute_sentinel_named_arguments);
return; return;

View File

@ -146,90 +146,74 @@ std::string Sema::getDeletedOrUnavailableSuffix(const FunctionDecl *FD) {
return std::string(); return std::string();
} }
/// DiagnoseSentinelCalls - This routine checks on method dispatch calls /// DiagnoseSentinelCalls - This routine checks whether a call or
/// (and other functions in future), which have been declared with sentinel /// message-send is to a declaration with the sentinel attribute, and
/// attribute. It warns if call does not have the sentinel argument. /// if so, it checks that the requirements of the sentinel are
/// /// satisfied.
void Sema::DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc, void Sema::DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc,
Expr **Args, unsigned NumArgs) { Expr **args, unsigned numArgs) {
const SentinelAttr *attr = D->getAttr<SentinelAttr>(); const SentinelAttr *attr = D->getAttr<SentinelAttr>();
if (!attr) if (!attr)
return; return;
int sentinelPos = attr->getSentinel(); // The number of formal parameters of the declaration.
int nullPos = attr->getNullPos(); unsigned numFormalParams;
// The kind of declaration. This is also an index into a %select in
// the diagnostic.
enum CalleeType { CT_Function, CT_Method, CT_Block } calleeType;
unsigned int i = 0;
bool warnNotEnoughArgs = false;
int isMethod = 0;
if (ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) { if (ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
// skip over named parameters. numFormalParams = MD->param_size();
ObjCMethodDecl::param_iterator P, E = MD->param_end(); calleeType = CT_Method;
for (P = MD->param_begin(); (P != E && i < NumArgs); ++P) {
if (nullPos)
--nullPos;
else
++i;
}
warnNotEnoughArgs = (P != E || i >= NumArgs);
isMethod = 1;
} else if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) { } else if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
// skip over named parameters. numFormalParams = FD->param_size();
ObjCMethodDecl::param_iterator P, E = FD->param_end(); calleeType = CT_Function;
for (P = FD->param_begin(); (P != E && i < NumArgs); ++P) { } else if (isa<VarDecl>(D)) {
if (nullPos) QualType type = cast<ValueDecl>(D)->getType();
--nullPos; const FunctionType *fn = 0;
else if (const PointerType *ptr = type->getAs<PointerType>()) {
++i; fn = ptr->getPointeeType()->getAs<FunctionType>();
} if (!fn) return;
warnNotEnoughArgs = (P != E || i >= NumArgs); calleeType = CT_Function;
} else if (VarDecl *V = dyn_cast<VarDecl>(D)) { } else if (const BlockPointerType *ptr = type->getAs<BlockPointerType>()) {
// block or function pointer call. fn = ptr->getPointeeType()->castAs<FunctionType>();
QualType Ty = V->getType(); calleeType = CT_Block;
if (Ty->isBlockPointerType() || Ty->isFunctionPointerType()) { } else {
const FunctionType *FT = Ty->isFunctionPointerType()
? Ty->getAs<PointerType>()->getPointeeType()->getAs<FunctionType>()
: Ty->getAs<BlockPointerType>()->getPointeeType()->getAs<FunctionType>();
if (const FunctionProtoType *Proto = dyn_cast<FunctionProtoType>(FT)) {
unsigned NumArgsInProto = Proto->getNumArgs();
unsigned k;
for (k = 0; (k != NumArgsInProto && i < NumArgs); k++) {
if (nullPos)
--nullPos;
else
++i;
}
warnNotEnoughArgs = (k != NumArgsInProto || i >= NumArgs);
}
if (Ty->isBlockPointerType())
isMethod = 2;
} else
return; return;
} else }
return;
if (warnNotEnoughArgs) { if (const FunctionProtoType *proto = dyn_cast<FunctionProtoType>(fn)) {
Diag(Loc, diag::warn_not_enough_argument) << D->getDeclName(); numFormalParams = proto->getNumArgs();
Diag(D->getLocation(), diag::note_sentinel_here) << isMethod; } else {
numFormalParams = 0;
}
} else {
return; return;
} }
int sentinel = i;
while (sentinelPos > 0 && i < NumArgs-1) { // "nullPos" is the number of formal parameters at the end which
--sentinelPos; // effectively count as part of the variadic arguments. This is
++i; // useful if you would prefer to not have *any* formal parameters,
} // but the language forces you to have at least one.
if (sentinelPos > 0) { unsigned nullPos = attr->getNullPos();
assert((nullPos == 0 || nullPos == 1) && "invalid null position on sentinel");
numFormalParams = (nullPos > numFormalParams ? 0 : numFormalParams - nullPos);
// The number of arguments which should follow the sentinel.
unsigned numArgsAfterSentinel = attr->getSentinel();
// If there aren't enough arguments for all the formal parameters,
// the sentinel, and the args after the sentinel, complain.
if (numArgs < numFormalParams + numArgsAfterSentinel + 1) {
Diag(Loc, diag::warn_not_enough_argument) << D->getDeclName(); Diag(Loc, diag::warn_not_enough_argument) << D->getDeclName();
Diag(D->getLocation(), diag::note_sentinel_here) << isMethod; Diag(D->getLocation(), diag::note_sentinel_here) << calleeType;
return; return;
} }
while (i < NumArgs-1) {
++i; // Otherwise, find the sentinel expression.
++sentinel; Expr *sentinelExpr = args[numArgs - numArgsAfterSentinel - 1];
}
Expr *sentinelExpr = Args[sentinel];
if (!sentinelExpr) return; if (!sentinelExpr) return;
if (sentinelExpr->isTypeDependent()) return;
if (sentinelExpr->isValueDependent()) return; if (sentinelExpr->isValueDependent()) return;
// nullptr_t is always treated as null. // nullptr_t is always treated as null.
@ -243,23 +227,25 @@ void Sema::DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc,
// Unfortunately, __null has type 'int'. // Unfortunately, __null has type 'int'.
if (isa<GNUNullExpr>(sentinelExpr)) return; if (isa<GNUNullExpr>(sentinelExpr)) return;
// Pick a reasonable string to insert. Optimistically use 'nil' or
// 'NULL' if those are actually defined in the context. Only use
// 'nil' for ObjC methods, where it's much more likely that the
// variadic arguments form a list of object pointers.
SourceLocation MissingNilLoc SourceLocation MissingNilLoc
= PP.getLocForEndOfToken(sentinelExpr->getLocEnd()); = PP.getLocForEndOfToken(sentinelExpr->getLocEnd());
std::string NullValue; std::string NullValue;
if (isMethod && PP.getIdentifierInfo("nil")->hasMacroDefinition()) if (calleeType == CT_Method &&
PP.getIdentifierInfo("nil")->hasMacroDefinition())
NullValue = "nil"; NullValue = "nil";
else if (PP.getIdentifierInfo("NULL")->hasMacroDefinition()) else if (PP.getIdentifierInfo("NULL")->hasMacroDefinition())
NullValue = "NULL"; NullValue = "NULL";
else if (Context.getTypeSize(Context.IntTy)
== Context.getTypeSize(Context.getSizeType()))
NullValue = "0";
else else
NullValue = "0L"; NullValue = "(void*) 0";
Diag(MissingNilLoc, diag::warn_missing_sentinel) Diag(MissingNilLoc, diag::warn_missing_sentinel)
<< isMethod << calleeType
<< FixItHint::CreateInsertion(MissingNilLoc, ", " + NullValue); << FixItHint::CreateInsertion(MissingNilLoc, ", " + NullValue);
Diag(D->getLocation(), diag::note_sentinel_here) << isMethod; Diag(D->getLocation(), diag::note_sentinel_here) << calleeType;
} }
SourceRange Sema::getExprRange(Expr *E) const { SourceRange Sema::getExprRange(Expr *E) const {