diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -720,6 +720,8 @@ def UnusedLabel : DiagGroup<"unused-label">; def UnusedLambdaCapture : DiagGroup<"unused-lambda-capture">; def UnusedParameter : DiagGroup<"unused-parameter">; +def UnusedButSetParameter : DiagGroup<"unused-but-set-parameter">; +def UnusedButSetVariable : DiagGroup<"unused-but-set-variable">; def UnusedResult : DiagGroup<"unused-result">; def PotentiallyEvaluatedExpression : DiagGroup<"potentially-evaluated-expression">; def UnevaluatedExpression : DiagGroup<"unevaluated-expression", diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -310,8 +310,12 @@ "repeated RISC-V 'interrupt' attribute is here">; def warn_unused_parameter : Warning<"unused parameter %0">, InGroup, DefaultIgnore; +def warn_unused_but_set_parameter : Warning<"parameter %0 set but not used">, + InGroup, DefaultIgnore; def warn_unused_variable : Warning<"unused variable %0">, InGroup, DefaultIgnore; +def warn_unused_but_set_variable : Warning<"variable %0 set but not used">, + InGroup, DefaultIgnore; def warn_unused_local_typedef : Warning< "unused %select{typedef|type alias}0 %1">, InGroup, DefaultIgnore; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2827,6 +2827,13 @@ /// ParmVarDecl pointers. void DiagnoseUnusedParameters(ArrayRef Parameters); + /// Diagnose any unused but set parameters in the given sequence of + /// ParmVarDecl pointers. + void DiagnoseUnusedButSetParameters(ArrayRef Parameters); + + /// Diagnose any unused but set variables declared in this CompoundStmt + void DiagnoseUnusedButSetVariables(CompoundStmt *CS); + /// 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. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -24,6 +24,7 @@ #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/NonTrivialTypeVisitor.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/StmtCXX.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/PartialDiagnostic.h" @@ -43,6 +44,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" #include "clang/Sema/Template.h" +#include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/Triple.h" #include @@ -13733,6 +13735,130 @@ } } +using AllUsesSetsPtrSet = llvm::SmallPtrSet; + +/// Remove any Decl in Set that is used in Body in any way other than the LHS of +/// an assignment. +static void AreAllUsesSets(Stmt *Body, AllUsesSetsPtrSet *Set) { + + struct AllUsesAreSetsVisitor : RecursiveASTVisitor { + AllUsesSetsPtrSet &S; + + AllUsesAreSetsVisitor(AllUsesSetsPtrSet &Set) : S(Set) {} + + bool TraverseBinaryOperator(const BinaryOperator *BO) { + auto *LHS = BO->getLHS(); + auto *DRE = dyn_cast(LHS); + if (!DRE || !S.count(DRE->getFoundDecl())) { + // The LHS is not a reference to one of our NamedDecls. + TraverseStmt(LHS); + } + TraverseStmt(BO->getRHS()); + return true; + } + + bool VisitDeclRefExpr(const DeclRefExpr *DRE) { + // If we remove all Decls, no need to keep searching. + return (!S.erase(DRE->getFoundDecl()) || S.size()); + } + }; + + if (!Set->size()) + return; + + AllUsesAreSetsVisitor Visitor(*Set); + Visitor.TraverseStmt(Body); +} + +template +static void DiagnoseUnusedButSetDecls(Sema *Se, Stmt *Body, R Decls, + unsigned DiagID) { + // Put the Decls in a set so we only have to traverse the body once for all of + // them. + AllUsesSetsPtrSet AllUsesAreSets; + + for (const NamedDecl *ND : Decls) { + AllUsesAreSets.insert(ND); + } + + AreAllUsesSets(Body, &AllUsesAreSets); + + for (auto Iter = AllUsesAreSets.begin(), End = AllUsesAreSets.end(); + Iter != End; ++Iter) { + Se->Diag((*Iter)->getLocation(), DiagID) << (*Iter)->getDeclName(); + } +} + +void Sema::DiagnoseUnusedButSetParameters(ArrayRef Parameters) { + // Don't diagnose unused-but-set-parameter errors in template instantiations; + // we will already have done so in the template itself. + if (inTemplateInstantiation()) + return; + + bool CPlusPlus = getLangOpts().CPlusPlus; + + auto IsCandidate = [&](const ParmVarDecl *P) { + // Check for Ignored here, because if we have no candidates we can avoid + // walking the AST. + if (Diags.getDiagnosticLevel(diag::warn_unused_but_set_parameter, + P->getLocation()) == + DiagnosticsEngine::Ignored) + return false; + if (!P->isReferenced() || !P->getDeclName() || P->hasAttr()) + return false; + // Mimic gcc's behavior regarding nonscalar types. + if (CPlusPlus && !P->getType()->isScalarType()) + return false; + return true; + }; + + auto Candidates = llvm::make_filter_range(Parameters, IsCandidate); + + if (Parameters.empty()) + return; + + Decl *D = Decl::castFromDeclContext((*Parameters.begin())->getDeclContext()); + Stmt *Body = D->getBody(); + if (Body) + DiagnoseUnusedButSetDecls(this, Body, Candidates, + diag::warn_unused_but_set_parameter); +} + +void Sema::DiagnoseUnusedButSetVariables(CompoundStmt *CS) { + bool CPlusPlus = getLangOpts().CPlusPlus; + + auto IsCandidate = [&](const Stmt *S) { + const DeclStmt *SD = dyn_cast(S); + if (!SD || !SD->isSingleDecl()) + return false; + const VarDecl *VD = dyn_cast(SD->getSingleDecl()); + // Check for Ignored here, because if we have no candidates we can avoid + // walking the AST. + if (!VD || Diags.getDiagnosticLevel(diag::warn_unused_but_set_variable, + VD->getLocation()) == + DiagnosticsEngine::Ignored) + return false; + if (!VD->isReferenced() || !VD->getDeclName() || VD->hasAttr()) + return false; + // Mimic gcc's behavior regarding nonscalar types. + if (CPlusPlus && !VD->getType()->isScalarType()) + return false; + return true; + }; + + auto Candidates = llvm::make_filter_range(CS->body(), IsCandidate); + + auto ToNamedDecl = [](const Stmt *S) { + const DeclStmt *SD = dyn_cast(S); + return dyn_cast(SD->getSingleDecl()); + }; + + auto CandidateDecls = llvm::map_range(Candidates, ToNamedDecl); + + DiagnoseUnusedButSetDecls(this, CS, CandidateDecls, + diag::warn_unused_but_set_variable); +} + void Sema::DiagnoseSizeOfParametersAndReturnValue( ArrayRef Parameters, QualType ReturnTy, NamedDecl *D) { if (LangOpts.NumLargeByValueCopy == 0) // No check. @@ -14435,8 +14561,10 @@ if (!FD->isInvalidDecl()) { // Don't diagnose unused parameters of defaulted or deleted functions. - if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody()) + if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody()) { DiagnoseUnusedParameters(FD->parameters()); + DiagnoseUnusedButSetParameters(FD->parameters()); + } DiagnoseSizeOfParametersAndReturnValue(FD->parameters(), FD->getReturnType(), FD); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -15498,6 +15498,9 @@ BD->setBody(cast(Body)); + // wait to diagnose unused but set parameters until after setBody + DiagnoseUnusedButSetParameters(BD->parameters()); + if (Body && getCurFunction()->HasPotentialAvailabilityViolations) DiagnoseUnguardedAvailabilityViolations(BD); diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -433,7 +433,9 @@ DiagnoseEmptyLoopBody(Elts[i], Elts[i + 1]); } - return CompoundStmt::Create(Context, Elts, L, R); + CompoundStmt *CS = CompoundStmt::Create(Context, Elts, L, R); + DiagnoseUnusedButSetVariables(CS); + return CS; } ExprResult diff --git a/clang/test/Sema/vector-gcc-compat.c b/clang/test/Sema/vector-gcc-compat.c --- a/clang/test/Sema/vector-gcc-compat.c +++ b/clang/test/Sema/vector-gcc-compat.c @@ -1,5 +1,8 @@ // RUN: %clang_cc1 %s -verify -fsyntax-only -Weverything -triple x86_64-apple-darwin10 +#pragma clang diagnostic ignored "-Wunused-but-set-parameter" +#pragma clang diagnostic ignored "-Wunused-but-set-variable" + // Test the compatibility of clang's vector extensions with gcc's vector // extensions for C. Notably &&, ||, ?: and ! are not available. typedef long long v2i64 __attribute__((vector_size(16))); diff --git a/clang/test/Sema/warn-unused-but-set-parameters-cpp.cpp b/clang/test/Sema/warn-unused-but-set-parameters-cpp.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Sema/warn-unused-but-set-parameters-cpp.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-parameter -verify %s + +int f0(int x, + int y, // expected-warning{{parameter 'y' set but not used}} + int z __attribute__((unused))) { + y = 0; + return x; +} + +void f1(void) { + (void)^(int x, + int y, // expected-warning{{parameter 'y' set but not used}} + int z __attribute__((unused))) { + y = 0; + return x; + }; +} + +struct S { + int i; +}; + +// in C++, don't warn for a struct (following gcc) +void f3(struct S s) { + struct S t; + s = t; +} diff --git a/clang/test/Sema/warn-unused-but-set-parameters.c b/clang/test/Sema/warn-unused-but-set-parameters.c new file mode 100644 --- /dev/null +++ b/clang/test/Sema/warn-unused-but-set-parameters.c @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-parameter -verify %s + +int f0(int x, + int y, // expected-warning{{parameter 'y' set but not used}} + int z __attribute__((unused))) { + y = 0; + return x; +} + +void f1(void) { + (void)^(int x, + int y, // expected-warning{{parameter 'y' set but not used}} + int z __attribute__((unused))) { + y = 0; + return x; + }; +} + +struct S { + int i; +}; + +void f3(struct S s) { // expected-warning{{parameter 's' set but not used}} + struct S t; + s = t; +} diff --git a/clang/test/Sema/warn-unused-but-set-variables-cpp.cpp b/clang/test/Sema/warn-unused-but-set-variables-cpp.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Sema/warn-unused-but-set-variables-cpp.cpp @@ -0,0 +1,33 @@ +// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-variable -verify %s + +struct S { + int i; +}; + +int f0() { + int y; // expected-warning{{variable 'y' set but not used}} + y = 0; + + int z __attribute__((unused)); + z = 0; + + // no warning for structs in C++ (following gcc's behavior) + struct S s; + struct S t; + s = t; + + int x; + x = 0; + return x; +} + +void f1(void) { + (void)^() { + int y; // expected-warning{{variable 'y' set but not used}} + y = 0; + + int x; + x = 0; + return x; + }; +} diff --git a/clang/test/Sema/warn-unused-but-set-variables.c b/clang/test/Sema/warn-unused-but-set-variables.c new file mode 100644 --- /dev/null +++ b/clang/test/Sema/warn-unused-but-set-variables.c @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-variable -verify %s + +struct S { + int i; +}; + +int f0() { + int y; // expected-warning{{variable 'y' set but not used}} + y = 0; + + int z __attribute__((unused)); + z = 0; + + struct S s; // expected-warning{{variable 's' set but not used}} + struct S t; + s = t; + + int x; + x = 0; + return x; +} + +void f1(void) { + (void)^() { + int y; // expected-warning{{variable 'y' set but not used}} + y = 0; + + int x; + x = 0; + return x; + }; +}