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,13 @@ void setReferenced(bool R = true) { Referenced = R; } + /// \brief Whether the declared symbol is written either directly or + /// indirectly. A "written" declaration can't be const. + 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 @@ -431,6 +431,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: + /// \brief 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 @@ -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: lib/Parse/ParseDeclCXX.cpp =================================================================== --- lib/Parse/ParseDeclCXX.cpp +++ lib/Parse/ParseDeclCXX.cpp @@ -3285,6 +3285,12 @@ return true; } + // Mark initializer parameters as written. + for (Expr *E : ArgExprs) { + if (auto *D = dyn_cast(E)) + D->getDecl()->setWritten(); + } + T.consumeClose(); SourceLocation EllipsisLoc; 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(ER.get())) { + if (B->isAssignmentOp() || B->isAdditiveOp()) { + 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 @@ -408,6 +423,9 @@ if (TernaryMiddle.isUsable()) TernaryMiddle = Actions.CorrectDelayedTyposInExpr(TernaryMiddle); LHS = ExprError(); + } else if (auto *B = dyn_cast(RHS.get())) { + if (B->isAssignmentOp()) + MarkWritten(B->getRHS()); } NextTokPrec = getBinOpPrecedence(Tok.getKind(), GreaterThanIsOperator, Index: lib/Parse/ParseExprCXX.cpp =================================================================== --- lib/Parse/ParseExprCXX.cpp +++ lib/Parse/ParseExprCXX.cpp @@ -2828,6 +2828,8 @@ if (Operand.isInvalid()) return Operand; + MarkWritten(Operand.get()); + return Actions.ActOnCXXDelete(Start, UseGlobal, ArrayDelete, Operand.get()); } Index: lib/Parse/ParseStmt.cpp =================================================================== --- lib/Parse/ParseStmt.cpp +++ lib/Parse/ParseStmt.cpp @@ -372,6 +372,84 @@ 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. + const FunctionDecl *FD = CE->getDirectCallee(); + if (!FD) { + // Mark all arguments as written + // TODO: Handle CXXMemberCallExpr better + for (auto *Arg : CE->arguments()) { + MarkWritten(Arg); + } + return; + } + int ArgNr = -1; + for (const auto *Par : FD->parameters()) { + ArgNr++; + if ((unsigned)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((unsigned)ArgNr); + if (ParType->isReferenceType()) + if (auto *U = dyn_cast(Arg)) + Arg = U->getSubExpr(); + 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 (auto *A = dyn_cast(E)) { + Deref = IncDec; + E = A->getBase()->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 +467,9 @@ return Actions.ActOnExprStmtError(); } + // Mark symbols in the expression as written. + MarkWritten(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 +1035,18 @@ StmtResult R; if (Tok.isNot(tok::kw___extension__)) { R = ParseStatementOrDeclaration(Stmts, false); + if (!R.isInvalid() && R.get()) { + if (ReturnStmt *RS = dyn_cast(R.get())) { + if (RS->getRetValue()) { + if (auto *Cast = dyn_cast(RS->getRetValue())) { + const Type *T = Cast->getType().getTypePtr(); + if (T->isPointerType() && !T->getPointeeType().isConstQualified()) { + MarkWritten(Cast); + } + } + } + } + } } else { // __extension__ can start declarations and it can also be a unary // operator for expressions. Consume multiple __extension__ markers here Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -8931,6 +8931,23 @@ } } +static void MarkWritten(Expr *E) { + E = E->IgnoreParenCasts(); + if (auto *B = dyn_cast(E)) { + if (B->isAdditiveOp()) { + MarkWritten(B->getLHS()); + MarkWritten(B->getRHS()); + } + } else 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. @@ -8943,6 +8960,8 @@ return; } + MarkWritten(Init); + if (CXXMethodDecl *Method = dyn_cast(RealDecl)) { // Pure-specifiers are handled in ActOnPureSpecifier. Diag(Method->getLocation(), diag::err_member_function_initialization) @@ -10356,6 +10375,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->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; + // 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, @@ -10928,6 +10982,8 @@ // 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); @@ -10936,7 +10992,7 @@ MarkVTableUsed(FD->getLocation(), Constructor->getParent()); else if (CXXDestructorDecl *Destructor = dyn_cast(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/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -9525,6 +9525,17 @@ } } +/// \brief 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, @@ -9535,6 +9546,8 @@ if (CheckForModifiableLvalue(LHSExpr, Loc, *this)) return QualType(); + MarkWritten(LHSExpr); + QualType LHSType = LHSExpr->getType(); QualType RHSType = CompoundType.isNull() ? RHS.get()->getType() : CompoundType; @@ -12818,7 +12831,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 @@ -482,6 +482,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 @@ -3631,6 +3631,8 @@ if (OldVar->isUsed(false)) NewVar->setIsUsed(); NewVar->setReferenced(OldVar->isReferenced()); + if (OldVar->isWritten()) + NewVar->setWritten(); } InstantiateAttrs(TemplateArgs, OldVar, NewVar, LateAttrs, StartingScope); 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;