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.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@139361 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index a77454e..624ac08 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -1771,7 +1771,7 @@
return;
}
- int sentinel = 0;
+ unsigned sentinel = 0;
if (Attr.getNumArgs() > 0) {
Expr *E = Attr.getArg(0);
llvm::APSInt Idx(32);
@@ -1781,16 +1781,17 @@
<< "sentinel" << 1 << E->getSourceRange();
return;
}
- sentinel = Idx.getZExtValue();
- if (sentinel < 0) {
+ if (Idx.isSigned() && Idx.isNegative()) {
S.Diag(Attr.getLoc(), diag::err_attribute_sentinel_less_than_zero)
<< E->getSourceRange();
return;
}
+
+ sentinel = Idx.getZExtValue();
}
- int nullPos = 0;
+ unsigned nullPos = 0;
if (Attr.getNumArgs() > 1) {
Expr *E = Attr.getArg(1);
llvm::APSInt Idx(32);
@@ -1802,7 +1803,7 @@
}
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
// to say what the bounds actually are.
S.Diag(Attr.getLoc(), diag::err_attribute_sentinel_not_zero_or_one)
@@ -1812,9 +1813,7 @@
}
if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
- const FunctionType *FT = FD->getType()->getAs<FunctionType>();
- assert(FT && "FunctionDecl has non-function type?");
-
+ const FunctionType *FT = FD->getType()->castAs<FunctionType>();
if (isa<FunctionNoProtoType>(FT)) {
S.Diag(Attr.getLoc(), diag::warn_attribute_sentinel_named_arguments);
return;
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index f10bb34..75e03c1 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -146,90 +146,74 @@
return std::string();
}
-/// DiagnoseSentinelCalls - This routine checks on method dispatch calls
-/// (and other functions in future), which have been declared with sentinel
-/// attribute. It warns if call does not have the sentinel argument.
-///
+/// DiagnoseSentinelCalls - This routine checks whether a call or
+/// message-send is to a declaration with the sentinel attribute, and
+/// if so, it checks that the requirements of the sentinel are
+/// satisfied.
void Sema::DiagnoseSentinelCalls(NamedDecl *D, SourceLocation Loc,
- Expr **Args, unsigned NumArgs) {
+ Expr **args, unsigned numArgs) {
const SentinelAttr *attr = D->getAttr<SentinelAttr>();
if (!attr)
return;
- int sentinelPos = attr->getSentinel();
- int nullPos = attr->getNullPos();
+ // The number of formal parameters of the declaration.
+ unsigned numFormalParams;
- unsigned int i = 0;
- bool warnNotEnoughArgs = false;
- int isMethod = 0;
+ // The kind of declaration. This is also an index into a %select in
+ // the diagnostic.
+ enum CalleeType { CT_Function, CT_Method, CT_Block } calleeType;
+
if (ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
- // skip over named parameters.
- ObjCMethodDecl::param_iterator P, E = MD->param_end();
- for (P = MD->param_begin(); (P != E && i < NumArgs); ++P) {
- if (nullPos)
- --nullPos;
- else
- ++i;
- }
- warnNotEnoughArgs = (P != E || i >= NumArgs);
- isMethod = 1;
+ numFormalParams = MD->param_size();
+ calleeType = CT_Method;
} else if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
- // skip over named parameters.
- ObjCMethodDecl::param_iterator P, E = FD->param_end();
- for (P = FD->param_begin(); (P != E && i < NumArgs); ++P) {
- if (nullPos)
- --nullPos;
- else
- ++i;
- }
- warnNotEnoughArgs = (P != E || i >= NumArgs);
- } else if (VarDecl *V = dyn_cast<VarDecl>(D)) {
- // block or function pointer call.
- QualType Ty = V->getType();
- if (Ty->isBlockPointerType() || Ty->isFunctionPointerType()) {
- 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
+ numFormalParams = FD->param_size();
+ calleeType = CT_Function;
+ } else if (isa<VarDecl>(D)) {
+ QualType type = cast<ValueDecl>(D)->getType();
+ const FunctionType *fn = 0;
+ if (const PointerType *ptr = type->getAs<PointerType>()) {
+ fn = ptr->getPointeeType()->getAs<FunctionType>();
+ if (!fn) return;
+ calleeType = CT_Function;
+ } else if (const BlockPointerType *ptr = type->getAs<BlockPointerType>()) {
+ fn = ptr->getPointeeType()->castAs<FunctionType>();
+ calleeType = CT_Block;
+ } else {
return;
- } else
- return;
+ }
- if (warnNotEnoughArgs) {
- Diag(Loc, diag::warn_not_enough_argument) << D->getDeclName();
- Diag(D->getLocation(), diag::note_sentinel_here) << isMethod;
+ if (const FunctionProtoType *proto = dyn_cast<FunctionProtoType>(fn)) {
+ numFormalParams = proto->getNumArgs();
+ } else {
+ numFormalParams = 0;
+ }
+ } else {
return;
}
- int sentinel = i;
- while (sentinelPos > 0 && i < NumArgs-1) {
- --sentinelPos;
- ++i;
- }
- if (sentinelPos > 0) {
+
+ // "nullPos" is the number of formal parameters at the end which
+ // effectively count as part of the variadic arguments. This is
+ // useful if you would prefer to not have *any* formal parameters,
+ // but the language forces you to have at least one.
+ 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(D->getLocation(), diag::note_sentinel_here) << isMethod;
+ Diag(D->getLocation(), diag::note_sentinel_here) << calleeType;
return;
}
- while (i < NumArgs-1) {
- ++i;
- ++sentinel;
- }
- Expr *sentinelExpr = Args[sentinel];
+
+ // Otherwise, find the sentinel expression.
+ Expr *sentinelExpr = args[numArgs - numArgsAfterSentinel - 1];
if (!sentinelExpr) return;
- if (sentinelExpr->isTypeDependent()) return;
if (sentinelExpr->isValueDependent()) return;
// nullptr_t is always treated as null.
@@ -243,23 +227,25 @@
// Unfortunately, __null has type 'int'.
if (isa<GNUNullExpr>(sentinelExpr)) return;
- SourceLocation MissingNilLoc
+ // 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
= PP.getLocForEndOfToken(sentinelExpr->getLocEnd());
std::string NullValue;
- if (isMethod && PP.getIdentifierInfo("nil")->hasMacroDefinition())
+ if (calleeType == CT_Method &&
+ PP.getIdentifierInfo("nil")->hasMacroDefinition())
NullValue = "nil";
else if (PP.getIdentifierInfo("NULL")->hasMacroDefinition())
NullValue = "NULL";
- else if (Context.getTypeSize(Context.IntTy)
- == Context.getTypeSize(Context.getSizeType()))
- NullValue = "0";
else
- NullValue = "0L";
+ NullValue = "(void*) 0";
Diag(MissingNilLoc, diag::warn_missing_sentinel)
- << isMethod
+ << calleeType
<< 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 {
