Index: include/clang/AST/DeclBase.h =================================================================== --- include/clang/AST/DeclBase.h +++ include/clang/AST/DeclBase.h @@ -273,6 +273,11 @@ /// are regarded as "referenced" but not "used". unsigned Referenced : 1; + /// \brief Whether the declared symbol is written. Either directly or + /// indirectly. This makes it possible to see if a nonconst declaration can + /// be "const". + unsigned Written : 1; + /// \brief Whether statistic collection is enabled. static bool StatisticsEnabled; @@ -324,26 +329,25 @@ bool AccessDeclContextSanity() const; protected: - Decl(Kind DK, DeclContext *DC, SourceLocation L) - : NextInContextAndBits(), DeclCtx(DC), - Loc(L), DeclKind(DK), InvalidDecl(0), - HasAttrs(false), Implicit(false), Used(false), Referenced(false), - Access(AS_none), FromASTFile(0), Hidden(DC && cast(DC)->Hidden), - IdentifierNamespace(getIdentifierNamespaceForKind(DK)), - CacheValidAndLinkage(0) - { - if (StatisticsEnabled) add(DK); + : NextInContextAndBits(), DeclCtx(DC), Loc(L), DeclKind(DK), + InvalidDecl(0), HasAttrs(false), Implicit(false), Used(false), + Referenced(false), Written(false), Access(AS_none), FromASTFile(0), + Hidden(DC && cast(DC)->Hidden), + IdentifierNamespace(getIdentifierNamespaceForKind(DK)), + CacheValidAndLinkage(0) { + if (StatisticsEnabled) + add(DK); } Decl(Kind DK, EmptyShell Empty) - : NextInContextAndBits(), DeclKind(DK), InvalidDecl(0), - HasAttrs(false), Implicit(false), Used(false), Referenced(false), - Access(AS_none), FromASTFile(0), Hidden(0), - IdentifierNamespace(getIdentifierNamespaceForKind(DK)), - CacheValidAndLinkage(0) - { - if (StatisticsEnabled) add(DK); + : NextInContextAndBits(), DeclKind(DK), InvalidDecl(0), HasAttrs(false), + Implicit(false), Used(false), Referenced(false), Written(false), + Access(AS_none), FromASTFile(0), Hidden(0), + IdentifierNamespace(getIdentifierNamespaceForKind(DK)), + CacheValidAndLinkage(0) { + if (StatisticsEnabled) + add(DK); } virtual ~Decl(); @@ -538,6 +542,12 @@ void setReferenced(bool R = true) { Referenced = R; } + /// \brief Whether the declared symbol is written. + bool isWritten() const { return Written; } + + /// \brief Set declaration as written. + void setWritten() { Written = Referenced; } + /// \brief Whether this declaration is a top-level declaration (function, /// global variable, etc.) that is lexically inside an objc container /// definition. Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -429,6 +429,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/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, DefaultIgnore; +def warn_nonconst_parameter : Warning<"parameter %0 can be const">, + InGroup, DefaultIgnore; def warn_unused_variable : Warning<"unused variable %0">, InGroup, DefaultIgnore; def warn_unused_local_typedef : Warning< Index: include/clang/Parse/Parser.h =================================================================== --- include/clang/Parse/Parser.h +++ include/clang/Parse/Parser.h @@ -1394,6 +1394,9 @@ bool IsUnevaluated); private: + // Mark symbols in expression as written. + static void MarkWritten(Expr *E); + ExprResult ParseExpressionWithLeadingAt(SourceLocation AtLoc); ExprResult ParseExpressionWithLeadingExtension(SourceLocation ExtLoc); Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -1758,6 +1758,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 DiagnoseNonConstParameters(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: lib/Parse/ParseExpr.cpp =================================================================== --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -167,7 +167,20 @@ 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(ER.get())) { + if (B->isAssignmentOp()) { + MarkWritten(B->getLHS()); + MarkWritten(B->getRHS()); + } + } else if (isa(ER.get()) || isa(ER.get()) || isa(ER.get())) { + MarkWritten(ER.get()); + } + return ER; } /// \brief Parse an assignment expression where part of an Objective-C message Index: lib/Parse/ParseStmt.cpp =================================================================== --- lib/Parse/ParseStmt.cpp +++ lib/Parse/ParseStmt.cpp @@ -372,6 +372,64 @@ return Res; } +// Mark symbols in r-value expression as written. +void Parser::MarkWritten(Expr *E) { + E = E->IgnoreParenCasts(); + if (auto *B = dyn_cast(E)) { + if (B->isAdditiveOp()) { + // p + 2 + MarkWritten(B->getLHS()); + MarkWritten(B->getRHS()); + } else if (B->isAssignmentOp()) { + // .. = p; + MarkWritten(B->getRHS()); + } + } else if (auto *CE = dyn_cast(E)) { + // Mark nonconst function parameters as written. + for (auto *Arg : CE->arguments()) { + // Is this a nonconstant pointer argument? + const Type *T = Arg->getType().getTypePtr(); + if (!T->isPointerType()) + continue; + if (T->getPointeeType().isConstQualified()) + continue; + // This is a nonconst pointer argument, the data is potentially written + // in the function. + MarkWritten(Arg); + } + } else if (auto *C = dyn_cast(E)) { + MarkWritten(C->getTrueExpr()); + MarkWritten(C->getFalseExpr()); + } else if (auto *D = dyn_cast(E)) { + D->getDecl()->setWritten(); + } else if (isa(E)) { + // ++(*p); + bool IncDec = false; + bool Deref = false; + while (auto *U = dyn_cast(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 (Deref) { + if (auto *D = dyn_cast(E)) + D->getDecl()->setWritten(); + } + } +} + /// \brief Parse an expression statement. StmtResult Parser::ParseExprStatement() { // If a case keyword is missing, this is where it should be inserted. @@ -389,6 +447,9 @@ return Actions.ActOnExprStmtError(); } + // Mark symbols in the expression as written. + MarkWritten(Expr.get()->IgnoreParenCasts()); + if (Tok.is(tok::colon) && getCurScope()->isSwitchScope() && Actions.CheckCaseExpression(Expr.get())) { // If a constant expression is followed by a colon inside a switch block, Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -8875,6 +8875,18 @@ } } +static void MarkWritten(Expr *E) { + E = E->IgnoreParenCasts(); + if (auto *C = dyn_cast(E)) { + MarkWritten(C->getFalseExpr()); + MarkWritten(C->getTrueExpr()); + } else if (auto *D = dyn_cast(E)) { + D->getDecl()->setWritten(); + } else if (auto *U = dyn_cast(E)) { + MarkWritten(U->getSubExpr()); + } +} + /// AddInitializerToDecl - Adds the initializer Init to the /// declaration dcl. If DirectInit is true, this is C++ direct /// initialization rather than copy initialization. @@ -8887,6 +8899,8 @@ return; } + MarkWritten(Init); + if (CXXMethodDecl *Method = dyn_cast(RealDecl)) { // Pure-specifiers are handled in ActOnPureSpecifier. Diag(Method->getLocation(), diag::err_member_function_initialization) @@ -10291,6 +10305,33 @@ } } +void Sema::DiagnoseNonConstParameters(ParmVarDecl *const *Param, + ParmVarDecl *const *ParamEnd) { + // Don't diagnose nonconst-parameter errors in template instantiations + if (!ActiveTemplateInstantiations.empty()) + return; + for (; Param != ParamEnd; ++Param) { + if (!(*Param)->isReferenced() || (*Param)->isWritten()) + 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; + // To start with we don't warn about structs. + if (T->getPointeeType().getTypePtr()->isRecordType()) + continue; + // To start with we don't warn about ** + if (T->getPointeeType().getTypePtr()->isPointerType()) + continue; + Diag((*Param)->getLocation(), diag::warn_nonconst_parameter) + << (*Param)->getDeclName(); + } +} + void Sema::DiagnoseSizeOfParametersAndReturnValue(ParmVarDecl * const *Param, ParmVarDecl * const *ParamEnd, QualType ReturnTy, @@ -10863,6 +10904,7 @@ // Don't diagnose unused parameters of defaulted or deleted functions. if (!FD->isDeleted() && !FD->isDefaulted()) DiagnoseUnusedParameters(FD->param_begin(), FD->param_end()); + DiagnoseNonConstParameters(FD->param_begin(), FD->param_end()); DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(), FD->param_end(), FD->getReturnType(), FD); Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -9503,6 +9503,17 @@ } } +// Mark symbols in l-value expression as written. +static void MarkWritten(Expr *E) { + E = E->IgnoreParenCasts(); + if (auto *D = dyn_cast(E)) + D->getDecl()->setWritten(); + else if (auto *U = dyn_cast(E)) + MarkWritten(U->getSubExpr()); + else if (auto *A = dyn_cast(E)) + MarkWritten(A->getBase()); +} + // C99 6.5.16.1 QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS, SourceLocation Loc, @@ -9513,6 +9524,8 @@ if (CheckForModifiableLvalue(LHSExpr, Loc, *this)) return QualType(); + MarkWritten(LHSExpr); + QualType LHSType = LHSExpr->getType(); QualType RHSType = CompoundType.isNull() ? RHS.get()->getType() : CompoundType; @@ -12782,7 +12795,8 @@ CopyExpr = new (S.Context) DeclRefExpr(Var, RefersToCapturedVariable, DeclRefType, VK_LValue, Loc); - Var->setReferenced(true); + Var->setReferenced(); + Var->setWritten(); Var->markUsed(S.Context); } Index: lib/Sema/SemaLambda.cpp =================================================================== --- lib/Sema/SemaLambda.cpp +++ lib/Sema/SemaLambda.cpp @@ -814,7 +814,8 @@ VarDecl *NewVD = VarDecl::Create(Context, CurContext, Loc, Loc, Id, InitCaptureType, TSI, SC_Auto); NewVD->setInitCapture(true); - NewVD->setReferenced(true); + NewVD->setReferenced(); + NewVD->setWritten(); NewVD->markUsed(Context); NewVD->setInit(Init); return NewVD; Index: lib/Sema/SemaOpenMP.cpp =================================================================== --- lib/Sema/SemaOpenMP.cpp +++ lib/Sema/SemaOpenMP.cpp @@ -466,6 +466,7 @@ SourceLocation Loc, bool RefersToCapture = false) { D->setReferenced(); + D->setWritten(); D->markUsed(S.Context); return DeclRefExpr::Create(S.getASTContext(), NestedNameSpecifierLoc(), SourceLocation(), D, RefersToCapture, Loc, Ty, Index: lib/Sema/SemaTemplateInstantiateDecl.cpp =================================================================== --- lib/Sema/SemaTemplateInstantiateDecl.cpp +++ lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -3613,6 +3613,7 @@ if (OldVar->isUsed(false)) NewVar->setIsUsed(); NewVar->setReferenced(OldVar->isReferenced()); + NewVar->setWritten(); } // See if the old variable had a type-specifier that defined an anonymous tag. Index: lib/Serialization/ASTReaderDecl.cpp =================================================================== --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -500,6 +500,7 @@ D->setImplicit(Record[Idx++]); D->Used = Record[Idx++]; D->setReferenced(Record[Idx++]); + D->setWritten(); D->setTopLevelDeclInObjCContainer(Record[Idx++]); D->setAccess((AccessSpecifier)Record[Idx++]); D->FromASTFile = true; Index: test/Sema/warn-nonconst-parameter.c =================================================================== --- test/Sema/warn-nonconst-parameter.c +++ test/Sema/warn-nonconst-parameter.c @@ -0,0 +1,102 @@ +// 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); + + +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); +} + +void dontwarn12(unsigned char *str, const unsigned int i) { + unsigned char *p; + for (p = str + i; *p; ) {} +} + +void dontwarn13(char *p) { + strcpy1(p, "abc"); +} + +void dontwarn14(char *p) { + strcpy1(p+2, "abc"); +} + +char *dontwarn15(char *p) { + return strcpy1(p, "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: test/SemaCXX/warn-nonconst-parameter.cpp =================================================================== --- test/SemaCXX/warn-nonconst-parameter.cpp +++ test/SemaCXX/warn-nonconst-parameter.cpp @@ -0,0 +1,17 @@ +// 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: + virtual int f(int*p){return 0;} +};