danielmarjamaki updated this revision to Diff 37223.
danielmarjamaki added a comment.
Minor cleanups and refactorings
http://reviews.llvm.org/D12359
Files:
include/clang/AST/Decl.h
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/AST/Decl.cpp
lib/Parse/ParseExpr.cpp
lib/Parse/ParseStmt.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaLambda.cpp
lib/Sema/SemaOpenMP.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
lib/Serialization/ASTReaderDecl.cpp
test/Sema/warn-nonconst-parameter.c
test/SemaCXX/warn-nonconst-parameter.cpp
Index: test/SemaCXX/warn-nonconst-parameter.cpp
===================================================================
--- test/SemaCXX/warn-nonconst-parameter.cpp
+++ test/SemaCXX/warn-nonconst-parameter.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fsyntax-only -Wnonconst-parameter -verify %s
+//
+// Test that -Wnonconst-parameter does not warn for virtual functions.
+//
+
+// expected-no-diagnostics
+
+class Base {
+public:
+ virtual int f(int*p);
+};
+
+class Derived : public Base {
+public:
+ // Don't warn on inherited virtual function
+ virtual int f(int *p) { return 0; }
+
+ // Don't warn on non-inherited virtual function
+ virtual int g(int *p) { return 0; }
+};
+
+class Fred {
+private:
+ void *X;
+public:
+ Fred(void *X) : X(X) {}
+ void dostuff();
+};
+
+static Fred& dontwarn1(void *P) {
+ return *static_cast<Fred*>(P);
+}
+
+static void dontwarn2(void *P) {
+ ((Fred*)P)->dostuff();
+}
+
+
+// Don't warn when data is passed to nonconst reference parameter
+void dostuff(int &x);
+void dontwarn19(int *p) {
+ dostuff(*p);
+}
+
Index: test/Sema/warn-nonconst-parameter.c
===================================================================
--- test/Sema/warn-nonconst-parameter.c
+++ test/Sema/warn-nonconst-parameter.c
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -fsyntax-only -Wnonconst-parameter -verify %s
+
+// Currently the -Wnonconst-parameter only warns about pointer arguments.
+//
+// It can be defined both that the data is const and that the pointer is const,
+// the -Wnonconst-parameter only checks if the data can be const-specified.
+//
+// It does not warn about pointers to records or function pointers.
+
+// Some external function where first argument is nonconst and second is const.
+char* strcpy1(char *dest, const char *src);
+unsigned my_strcpy(char *buf, const char *s);
+unsigned my_strlen(const char *buf);
+
+
+int warn1(int *p) { // expected-warning {{parameter 'p' can be const}}
+ return *p;
+}
+
+void warn2(int *first, int *last) { // expected-warning {{parameter 'last' can be const}}
+ *first = 0;
+ if (first < last) {} // <- last can be const
+}
+
+void warn3(char *p) { // expected-warning {{parameter 'p' can be const}}
+ char buf[10];
+ strcpy1(buf, p);
+}
+
+int dontwarn1(const int *p) {
+ return *p;
+}
+
+void dontwarn2(int *p) {
+ *p = 0;
+}
+
+void dontwarn3(char *p) {
+ p[0] = 0;
+}
+
+void dontwarn4(int *p) {
+ int *q = p;
+ *q = 0;
+}
+
+void dontwarn5(float *p) {
+ int *q = (int *)p;
+}
+
+void dontwarn6(char *p) {
+ char *a, *b;
+ a = b = p;
+}
+
+void dontwarn7(int *p) {
+ int *i = p ? p : 0;
+}
+
+void dontwarn8(char *p) {
+ char *x;
+ x = (p ? p : "");
+}
+
+char *dontwarn9(char *p) {
+ char *x;
+ return p ? p : "";
+}
+
+void dontwarn10(int *p) {
+ ++(*p);
+}
+
+char dontwarn11(int *p) {
+ return ++(*p);
+}
+
+char *dontwarn12(char *s) {
+ return s;
+}
+
+void dontwarn13(unsigned char *str, const unsigned int i) {
+ unsigned char *p;
+ for (p = str + i; *p; ) {}
+}
+
+void dontwarn14(int *buf) {
+ int i, *p;
+ for (i=0, p=buf; i<10; i++, p++) {
+ *p = 1;
+ }
+}
+
+void dontwarn15(int *p, int x) {
+ for (int *q = p+x-1; 0; q++);
+}
+
+void dontwarn16(char *p) {
+ strcpy1(p, "abc");
+}
+
+void dontwarn17(char *p) {
+ strcpy1(p+2, "abc");
+}
+
+
+char *dontwarn18(char *p) {
+ return strcpy1(p, "abc");
+}
+
+unsigned dontwarn19(char *buf) {
+ unsigned len = my_strlen(buf);
+ return len + my_strcpy(buf, "abc");
+}
+
+// Don't warn about nonconst function pointers that can be const.
+void functionpointer(double f(double), int x) {
+ f(x);
+}
+
+// Don't warn about nonconst record pointers that can be const.
+struct XY { int x; int y; };
+int recordpointer(struct XY *xy) {
+ return xy->x;
+}
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -501,6 +501,8 @@
D->setImplicit(Record[Idx++]);
D->Used = Record[Idx++];
D->setReferenced(Record[Idx++]);
+ if (auto *VD = dyn_cast<VarDecl>(D))
+ VD->setNonConstUse();
D->setTopLevelDeclInObjCContainer(Record[Idx++]);
D->setAccess((AccessSpecifier)Record[Idx++]);
D->FromASTFile = true;
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3632,6 +3632,7 @@
if (OldVar->isUsed(false))
NewVar->setIsUsed();
NewVar->setReferenced(OldVar->isReferenced());
+ NewVar->setNonConstUse();
}
InstantiateAttrs(TemplateArgs, OldVar, NewVar, LateAttrs, StartingScope);
Index: lib/Sema/SemaOpenMP.cpp
===================================================================
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -499,6 +499,7 @@
SourceLocation Loc,
bool RefersToCapture = false) {
D->setReferenced();
+ D->setNonConstUse();
D->markUsed(S.Context);
return DeclRefExpr::Create(S.getASTContext(), NestedNameSpecifierLoc(),
SourceLocation(), D, RefersToCapture, Loc, Ty,
Index: lib/Sema/SemaLambda.cpp
===================================================================
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -815,7 +815,8 @@
VarDecl *NewVD = VarDecl::Create(Context, CurContext, Loc,
Loc, Id, InitCaptureType, TSI, SC_Auto);
NewVD->setInitCapture(true);
- NewVD->setReferenced(true);
+ NewVD->setReferenced();
+ NewVD->setNonConstUse();
NewVD->markUsed(Context);
NewVD->setInit(Init);
return NewVD;
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9492,6 +9492,19 @@
}
}
+/// \brief Set NonConstUse for variable declarations used in expression.
+static void MarkNonConstUse(Expr *E) {
+ E = E->IgnoreParenCasts();
+ if (auto *D = dyn_cast<DeclRefExpr>(E)) {
+ if (auto *VD = dyn_cast<VarDecl>(D->getDecl()))
+ VD->setNonConstUse();
+ } else if (auto *U = dyn_cast<UnaryOperator>(E)) {
+ MarkNonConstUse(U->getSubExpr());
+ } else if (auto *A = dyn_cast<ArraySubscriptExpr>(E)) {
+ MarkNonConstUse(A->getBase());
+ }
+}
+
// C99 6.5.16.1
QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
SourceLocation Loc,
@@ -9502,6 +9515,8 @@
if (CheckForModifiableLvalue(LHSExpr, Loc, *this))
return QualType();
+ MarkNonConstUse(LHSExpr);
+
QualType LHSType = LHSExpr->getType();
QualType RHSType = CompoundType.isNull() ? RHS.get()->getType() :
CompoundType;
@@ -12807,7 +12822,8 @@
CopyExpr = new (S.Context) DeclRefExpr(Var, RefersToCapturedVariable,
DeclRefType, VK_LValue, Loc);
- Var->setReferenced(true);
+ Var->setReferenced();
+ Var->setNonConstUse();
Var->markUsed(S.Context);
}
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8975,6 +8975,24 @@
}
}
+static void MarkNonConstUse(Expr *E) {
+ E = E->IgnoreParenCasts();
+ if (auto *B = dyn_cast<BinaryOperator>(E)) {
+ if (B->isAdditiveOp()) {
+ MarkNonConstUse(B->getLHS());
+ MarkNonConstUse(B->getRHS());
+ }
+ } else if (auto *C = dyn_cast<ConditionalOperator>(E)) {
+ MarkNonConstUse(C->getFalseExpr());
+ MarkNonConstUse(C->getTrueExpr());
+ } else if (auto *D = dyn_cast<DeclRefExpr>(E)) {
+ if (auto *VD = dyn_cast<VarDecl>(D->getDecl()))
+ VD->setNonConstUse();
+ } else if (auto *U = dyn_cast<UnaryOperator>(E)) {
+ MarkNonConstUse(U->getSubExpr());
+ }
+}
+
/// AddInitializerToDecl - Adds the initializer Init to the
/// declaration dcl. If DirectInit is true, this is C++ direct
/// initialization rather than copy initialization.
@@ -8987,6 +9005,8 @@
return;
}
+ MarkNonConstUse(Init);
+
if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(RealDecl)) {
// Pure-specifiers are handled in ActOnPureSpecifier.
Diag(Method->getLocation(), diag::err_member_function_initialization)
@@ -10398,6 +10418,41 @@
}
}
+void Sema::DiagnoseNonConstPointerParameters(ParmVarDecl * const *ParamBegin,
+ ParmVarDecl * const *ParamEnd) {
+ // Don't diagnose nonconst-parameter errors in template instantiations
+ if (!ActiveTemplateInstantiations.empty())
+ return;
+ for (const auto *Param : llvm::make_range(ParamBegin, ParamEnd)) {
+ if (!Param->isReferenced() || Param->hasNonConstUse())
+ continue;
+ const Type *T = Param->getType().getTypePtr();
+ if (!T->isPointerType())
+ continue;
+ if (T->getPointeeType().isConstQualified())
+ continue;
+ // Don't warn about function pointers.
+ if (T->getPointeeType().getTypePtr()->isFunctionProtoType())
+ continue;
+ // Don't warn about struct parameters. These are not handled properly by
+ // the code currently. There should not be warnings always when a struct
+ // pointer parameter can be const neither.
+ if (T->getPointeeType().getTypePtr()->isRecordType())
+ continue;
+ // Don't warn about ** parameters. These are not handled properly by the
+ // code currently.
+ if (T->getPointeeType().getTypePtr()->isPointerType())
+ continue;
+ // Don't warn about void * as such parameters are often used in casts that
+ // are hard to handle well. For now there are no warnings about such
+ // parameters.
+ if (T->getPointeeType().getTypePtr()->isVoidType())
+ continue;
+ Diag(Param->getLocation(), diag::warn_nonconst_parameter)
+ << Param->getDeclName();
+ }
+}
+
void Sema::DiagnoseSizeOfParametersAndReturnValue(ParmVarDecl * const *Param,
ParmVarDecl * const *ParamEnd,
QualType ReturnTy,
@@ -10970,15 +11025,17 @@
// Don't diagnose unused parameters of defaulted or deleted functions.
if (!FD->isDeleted() && !FD->isDefaulted())
DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
+ if (!getLangOpts().CPlusPlus)
+ DiagnoseNonConstPointerParameters(FD->param_begin(), FD->param_end());
DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(), FD->param_end(),
FD->getReturnType(), FD);
// If this is a structor, we need a vtable.
if (CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(FD))
MarkVTableUsed(FD->getLocation(), Constructor->getParent());
else if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(FD))
MarkVTableUsed(FD->getLocation(), Destructor->getParent());
-
+
// Try to apply the named return value optimization. We have to check
// if we can do this here because lambdas keep return statements around
// to deduce an implicit return type.
Index: lib/Parse/ParseStmt.cpp
===================================================================
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -372,6 +372,87 @@
return Res;
}
+// Mark symbols in r-value expression as written.
+void Parser::MarkNonConstUse(Expr *E) {
+ E = E->IgnoreParenCasts();
+ if (auto *B = dyn_cast<BinaryOperator>(E)) {
+ if (B->isAdditiveOp()) {
+ // p + 2
+ MarkNonConstUse(B->getLHS());
+ MarkNonConstUse(B->getRHS());
+ } else if (B->isAssignmentOp()) {
+ // .. = p;
+ MarkNonConstUse(B->getRHS());
+ }
+ } else if (auto *CE = dyn_cast<CallExpr>(E)) {
+ // Mark nonconst function parameters as written.
+ const FunctionDecl *FD = CE->getDirectCallee();
+ if (!FD) {
+ // Mark all arguments as written
+ // TODO: Handle CXXMemberCallExpr better
+ for (auto *Arg : CE->arguments()) {
+ MarkNonConstUse(Arg);
+ }
+ return;
+ }
+ unsigned ArgNr = 0U;
+ for (const auto *Par : FD->parameters()) {
+ if (ArgNr >= CE->getNumArgs())
+ break;
+ // Is this a nonconstant pointer/reference parameter?
+ const Type *ParType = Par->getType().getTypePtr();
+ if ((ParType->isPointerType() && !ParType->getPointeeType().isConstQualified()) ||
+ (ParType->isReferenceType() && !Par->getType().isConstQualified())) {
+ // This is a nonconst pointer/reference parameter, the data is
+ // potentially written in the function.
+ Expr *Arg = CE->getArg(ArgNr);
+ if (ParType->isReferenceType())
+ if (auto *U = dyn_cast<UnaryOperator>(Arg))
+ Arg = U->getSubExpr();
+ MarkNonConstUse(Arg);
+ }
+ ArgNr++;
+ }
+ } else if (auto *C = dyn_cast<ConditionalOperator>(E)) {
+ MarkNonConstUse(C->getTrueExpr());
+ MarkNonConstUse(C->getFalseExpr());
+ } else if (auto *D = dyn_cast<DeclRefExpr>(E)) {
+ if (auto *VD = dyn_cast<VarDecl>(D->getDecl()))
+ VD->setNonConstUse();
+ } else if (isa<UnaryOperator>(E)) {
+ // ++(*p);
+ bool IncDec = false;
+ bool Deref = false;
+ while (auto *U = dyn_cast<UnaryOperator>(E)) {
+ switch (U->getOpcode()) {
+ case UO_PreInc:
+ case UO_PostInc:
+ case UO_PreDec:
+ case UO_PostDec:
+ IncDec = true;
+ break;
+ case UO_Deref:
+ if (IncDec)
+ Deref = true;
+ break;
+ default:
+ break;
+ };
+ E = U->getSubExpr()->IgnoreParenCasts();
+ }
+ if (auto *A = dyn_cast<ArraySubscriptExpr>(E)) {
+ Deref = IncDec;
+ E = A->getBase()->IgnoreParenCasts();
+ }
+ if (Deref) {
+ if (auto *D = dyn_cast<DeclRefExpr>(E)) {
+ if (auto *VD = dyn_cast<VarDecl>(D->getDecl()))
+ VD->setNonConstUse();
+ }
+ }
+ }
+}
+
/// \brief Parse an expression statement.
StmtResult Parser::ParseExprStatement() {
// If a case keyword is missing, this is where it should be inserted.
@@ -389,6 +470,9 @@
return Actions.ActOnExprStmtError();
}
+ // Mark symbols in the expression as written.
+ MarkNonConstUse(Expr.get());
+
if (Tok.is(tok::colon) && getCurScope()->isSwitchScope() &&
Actions.CheckCaseExpression(Expr.get())) {
// If a constant expression is followed by a colon inside a switch block,
@@ -954,6 +1038,18 @@
StmtResult R;
if (Tok.isNot(tok::kw___extension__)) {
R = ParseStatementOrDeclaration(Stmts, false);
+ if (!R.isInvalid() && R.get()) {
+ if (ReturnStmt *RS = dyn_cast<ReturnStmt>(R.get())) {
+ if (RS->getRetValue()) {
+ if (auto *Cast = dyn_cast<ImplicitCastExpr>(RS->getRetValue())) {
+ const Type *T = Cast->getType().getTypePtr();
+ if (T->isPointerType() && !T->getPointeeType().isConstQualified()) {
+ MarkNonConstUse(Cast);
+ }
+ }
+ }
+ }
+ }
} else {
// __extension__ can start declarations and it can also be a unary
// operator for expressions. Consume multiple __extension__ markers here
Index: lib/Parse/ParseExpr.cpp
===================================================================
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -167,7 +167,22 @@
ExprResult LHS = ParseCastExpression(/*isUnaryExpression=*/false,
/*isAddressOfOperand=*/false,
isTypeCast);
- return ParseRHSOfBinaryExpression(LHS, prec::Assignment);
+
+ ExprResult ER(ParseRHSOfBinaryExpression(LHS, prec::Assignment));
+ if (ER.isInvalid())
+ return ER;
+
+ if (auto *B = dyn_cast<BinaryOperator>(ER.get())) {
+ if (B->isAssignmentOp() || B->isAdditiveOp()) {
+ MarkNonConstUse(B->getLHS());
+ MarkNonConstUse(B->getRHS());
+ }
+ } else if (isa<CallExpr>(ER.get()) ||
+ isa<ConditionalOperator>(ER.get()) ||
+ isa<UnaryOperator>(ER.get())) {
+ MarkNonConstUse(ER.get());
+ }
+ return ER;
}
/// \brief Parse an assignment expression where part of an Objective-C message
@@ -408,6 +423,9 @@
if (TernaryMiddle.isUsable())
TernaryMiddle = Actions.CorrectDelayedTyposInExpr(TernaryMiddle);
LHS = ExprError();
+ } else if (auto *B = dyn_cast<BinaryOperator>(RHS.get())) {
+ if (B->isAssignmentOp())
+ MarkNonConstUse(B->getRHS());
}
NextTokPrec = getBinOpPrecedence(Tok.getKind(), GreaterThanIsOperator,
Index: lib/AST/Decl.cpp
===================================================================
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1762,7 +1762,7 @@
IdentifierInfo *Id, QualType T, TypeSourceInfo *TInfo,
StorageClass SC)
: DeclaratorDecl(DK, DC, IdLoc, Id, T, TInfo, StartLoc),
- redeclarable_base(C), Init() {
+ redeclarable_base(C), Init(), NonConstUse(false) {
static_assert(sizeof(VarDeclBitfields) <= sizeof(unsigned),
"VarDeclBitfields too large!");
static_assert(sizeof(ParmVarDeclBitfields) <= sizeof(unsigned),
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1762,6 +1762,11 @@
void DiagnoseUnusedParameters(ParmVarDecl * const *Begin,
ParmVarDecl * const *End);
+ /// \brief Diagnose pointer parameters that should be const in the given
+ /// sequence of ParmVarDecl pointers.
+ void DiagnoseNonConstPointerParameters(ParmVarDecl * const *Begin,
+ ParmVarDecl * const *End);
+
/// \brief Diagnose whether the size of parameters or return value of a
/// function or obj-c method definition is pass-by-value and larger than a
/// specified threshold.
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -1395,6 +1395,10 @@
bool IsUnevaluated);
private:
+ /// \brief Mark variable declarations in expression as nonconstuse, to prevent
+ /// warnings that they can be const.
+ static void MarkNonConstUse(Expr *E);
+
ExprResult ParseExpressionWithLeadingAt(SourceLocation AtLoc);
ExprResult ParseExpressionWithLeadingExtension(SourceLocation ExtLoc);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -197,6 +197,8 @@
def err_parameter_name_omitted : Error<"parameter name omitted">;
def warn_unused_parameter : Warning<"unused parameter %0">,
InGroup<UnusedParameter>, DefaultIgnore;
+def warn_nonconst_parameter : Warning<"parameter %0 can be const">,
+ InGroup<NonConstParameter>, DefaultIgnore;
def warn_unused_variable : Warning<"unused variable %0">,
InGroup<UnusedVariable>, DefaultIgnore;
def warn_unused_local_typedef : Warning<
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -433,6 +433,7 @@
def UnusedLabel : DiagGroup<"unused-label">;
def UnusedParameter : DiagGroup<"unused-parameter">;
def UnusedResult : DiagGroup<"unused-result">;
+def NonConstParameter : DiagGroup<"nonconst-parameter">;
def PotentiallyEvaluatedExpression : DiagGroup<"potentially-evaluated-expression">;
def UnevaluatedExpression : DiagGroup<"unevaluated-expression",
[PotentiallyEvaluatedExpression]>;
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -850,6 +850,9 @@
return getMostRecentDecl();
}
+ /// \brief Whether this variable has non-const use so it can't be const.
+ unsigned NonConstUse:1;
+
public:
typedef redeclarable_base::redecl_range redecl_range;
typedef redeclarable_base::redecl_iterator redecl_iterator;
@@ -865,6 +868,13 @@
IdentifierInfo *Id, QualType T, TypeSourceInfo *TInfo,
StorageClass S);
+ /// \brief Whether the declared symbol has non-const usage so it can't be
+ /// const.
+ bool hasNonConstUse() const { return NonConstUse; }
+
+ /// \brief Use this method when this symbol can't be const.
+ void setNonConstUse() { NonConstUse = isThisDeclarationReferenced(); }
+
static VarDecl *CreateDeserialized(ASTContext &C, unsigned ID);
SourceRange getSourceRange() const override LLVM_READONLY;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
