[Sema] Patch to issue warning on comparing parameters with
nonnull attribute when comparison is always true/false.
Original patch by Steven Wu. I have added extra code to prevent issuing of
warning when the nonnull parameter is modified prior to the comparison.
This addition prevents false positives in the most obvious cases.
There may still be false positive warnings in some cases (as one of my tests
indicates), but benefit far outweighs such cases. rdar://18712242
llvm-svn: 222264
diff --git a/clang/lib/Sema/ScopeInfo.cpp b/clang/lib/Sema/ScopeInfo.cpp
index c4bf67b..00d9982 100644
--- a/clang/lib/Sema/ScopeInfo.cpp
+++ b/clang/lib/Sema/ScopeInfo.cpp
@@ -39,6 +39,7 @@
ErrorTrap.reset();
PossiblyUnreachableDiags.clear();
WeakObjectUses.clear();
+ ModifiedNonNullParams.clear();
}
static const NamedDecl *getBestPropertyDecl(const ObjCPropertyRefExpr *PropE) {
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 72fc3e3..aa6bf17 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -6739,7 +6739,39 @@
// Weak Decls can be null.
if (!D || D->isWeak())
return;
-
+
+ // Check for parameter decl with nonnull attribute
+ if (const ParmVarDecl* PV = dyn_cast<ParmVarDecl>(D)) {
+ if (getCurFunction() && !getCurFunction()->ModifiedNonNullParams.count(PV))
+ if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(PV->getDeclContext())) {
+ unsigned NumArgs = FD->getNumParams();
+ llvm::SmallBitVector AttrNonNull(NumArgs);
+ for (const auto *NonNull : FD->specific_attrs<NonNullAttr>()) {
+ if (!NonNull->args_size()) {
+ AttrNonNull.set(0, NumArgs);
+ break;
+ }
+ for (unsigned Val : NonNull->args()) {
+ if (Val >= NumArgs)
+ continue;
+ AttrNonNull.set(Val);
+ }
+ }
+ if (!AttrNonNull.empty())
+ for (unsigned i = 0; i < NumArgs; ++i)
+ if (FD->getParamDecl(i) == PV && AttrNonNull[i]) {
+ std::string Str;
+ llvm::raw_string_ostream S(Str);
+ E->printPretty(S, nullptr, getPrintingPolicy());
+ unsigned DiagID = IsCompare ? diag::warn_nonnull_parameter_compare
+ : diag::warn_cast_nonnull_to_bool;
+ Diag(E->getExprLoc(), DiagID) << S.str() << E->getSourceRange()
+ << Range << IsEqual;
+ return;
+ }
+ }
+ }
+
QualType T = D->getType();
const bool IsArray = T->isArrayType();
const bool IsFunction = T->isFunctionType();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 1429df7..0104020 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9121,6 +9121,24 @@
return Context.getPointerType(op->getType());
}
+static void RecordModifiableNonNullParam(Sema &S, const Expr *Exp) {
+ const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp);
+ if (!DRE)
+ return;
+ const Decl *D = DRE->getDecl();
+ if (!D)
+ return;
+ const ParmVarDecl *Param = dyn_cast<ParmVarDecl>(D);
+ if (!Param)
+ return;
+ if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(Param->getDeclContext()))
+ if (!FD->hasAttr<NonNullAttr>())
+ return;
+ if (FunctionScopeInfo *FD = S.getCurFunction())
+ if (!FD->ModifiedNonNullParams.count(Param))
+ FD->ModifiedNonNullParams.insert(Param);
+}
+
/// CheckIndirectionOperand - Type check unary indirection (prefix '*').
static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK,
SourceLocation OpLoc) {
@@ -9360,6 +9378,7 @@
}
if (!ResultTy.isNull())
DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc);
+ RecordModifiableNonNullParam(*this, LHS.get());
break;
case BO_PtrMemD:
case BO_PtrMemI:
@@ -9833,6 +9852,7 @@
break;
case UO_AddrOf:
resultType = CheckAddressOfOperand(Input, OpLoc);
+ RecordModifiableNonNullParam(*this, InputExpr);
break;
case UO_Deref: {
Input = DefaultFunctionArrayLvalueConversion(Input.get());