mbenfield updated this revision to Diff 338423.
mbenfield added a comment.
Fixed misfirings reported by aeubanks and added tests for those cases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100581/new/
https://reviews.llvm.org/D100581
Files:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/Sema/vector-gcc-compat.c
clang/test/Sema/warn-unused-but-set-parameters-cpp.cpp
clang/test/Sema/warn-unused-but-set-parameters.c
clang/test/Sema/warn-unused-but-set-variables-cpp.cpp
clang/test/Sema/warn-unused-but-set-variables.c
Index: clang/test/Sema/warn-unused-but-set-variables.c
===================================================================
--- /dev/null
+++ 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;
+ };
+}
Index: clang/test/Sema/warn-unused-but-set-variables-cpp.cpp
===================================================================
--- /dev/null
+++ 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;
+
+ // In C++, don't warn for structs. (following gcc's behavior)
+ struct S s;
+ struct S t;
+ s = t;
+
+ int x;
+ x = 0;
+ return x + 5;
+}
+
+void f1(void) {
+ (void)^() {
+ int y; // expected-warning{{variable 'y' set but not used}}
+ y = 0;
+
+ int x;
+ x = 0;
+ return x;
+ };
+}
Index: clang/test/Sema/warn-unused-but-set-parameters.c
===================================================================
--- /dev/null
+++ 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;
+}
Index: clang/test/Sema/warn-unused-but-set-parameters-cpp.cpp
===================================================================
--- /dev/null
+++ clang/test/Sema/warn-unused-but-set-parameters-cpp.cpp
@@ -0,0 +1,33 @@
+// 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;
+}
+
+// Make sure this doesn't warn.
+struct A {
+ int i;
+ A(int j) : i(j) {}
+};
Index: clang/test/Sema/vector-gcc-compat.c
===================================================================
--- clang/test/Sema/vector-gcc-compat.c
+++ 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)));
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -434,7 +434,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
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15498,6 +15498,9 @@
BD->setBody(cast<CompoundStmt>(Body));
+ // wait to diagnose unused but set parameters until after setBody
+ DiagnoseUnusedButSetParameters(BD->parameters());
+
if (Body && getCurFunction()->HasPotentialAvailabilityViolations)
DiagnoseUnguardedAvailabilityViolations(BD);
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ 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 <algorithm>
@@ -13733,6 +13735,133 @@
}
}
+using AllUsesSetsPtrSet = llvm::SmallPtrSet<const NamedDecl *, 16>;
+
+namespace {
+
+struct AllUsesAreSetsVisitor : RecursiveASTVisitor<AllUsesAreSetsVisitor> {
+ AllUsesSetsPtrSet &S;
+
+ AllUsesAreSetsVisitor(AllUsesSetsPtrSet &Set) : S(Set) {}
+
+ bool TraverseBinaryOperator(const BinaryOperator *BO) {
+ auto *LHS = BO->getLHS();
+ auto *DRE = dyn_cast<DeclRefExpr>(LHS);
+ if (!BO->isAssignmentOp() || !DRE || !S.count(DRE->getFoundDecl())) {
+ // This is not an assignment 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());
+ }
+
+ bool OverloadedTraverse(Stmt *S) { return TraverseStmt(S); }
+
+ bool OverloadedTraverse(Decl *D) { return TraverseDecl(D); }
+};
+
+} // end anonymous namespace
+
+/// For any NamedDecl in Decls that is not used in any way other than the LHS of
+/// an assignment, diagnose with the given DiagId.
+template <typename R, typename T>
+static void DiagnoseUnusedButSetDecls(Sema *Se, T *Parent, 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);
+ }
+
+ if (!AllUsesAreSets.size())
+ return;
+
+ AllUsesAreSetsVisitor Visitor(AllUsesAreSets);
+ Visitor.OverloadedTraverse(Parent);
+
+ for (const NamedDecl *ND : AllUsesAreSets) {
+ Se->Diag(ND->getLocation(), DiagID) << ND->getDeclName();
+ }
+}
+
+void Sema::DiagnoseUnusedButSetParameters(ArrayRef<ParmVarDecl *> 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<UnusedAttr>())
+ 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;
+
+ // Don't traverse just the body, because we'd miss things like
+ // CXXCtorInitializer.
+ Decl *D = Decl::castFromDeclContext((*Parameters.begin())->getDeclContext());
+ if (D)
+ DiagnoseUnusedButSetDecls(this, D, 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<DeclStmt>(S);
+ if (!SD || !SD->isSingleDecl())
+ return false;
+ const VarDecl *VD = dyn_cast<VarDecl>(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<UnusedAttr>())
+ 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<const DeclStmt>(S);
+ return dyn_cast<const NamedDecl>(SD->getSingleDecl());
+ };
+
+ auto CandidateDecls = llvm::map_range(Candidates, ToNamedDecl);
+
+ DiagnoseUnusedButSetDecls(this, CS, CandidateDecls,
+ diag::warn_unused_but_set_variable);
+}
+
void Sema::DiagnoseSizeOfParametersAndReturnValue(
ArrayRef<ParmVarDecl *> Parameters, QualType ReturnTy, NamedDecl *D) {
if (LangOpts.NumLargeByValueCopy == 0) // No check.
@@ -14435,8 +14564,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);
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2828,6 +2828,13 @@
/// ParmVarDecl pointers.
void DiagnoseUnusedParameters(ArrayRef<ParmVarDecl *> Parameters);
+ /// Diagnose any unused but set parameters in the given sequence of
+ /// ParmVarDecl pointers.
+ void DiagnoseUnusedButSetParameters(ArrayRef<ParmVarDecl *> 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.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ 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<UnusedParameter>, DefaultIgnore;
+def warn_unused_but_set_parameter : Warning<"parameter %0 set but not used">,
+ InGroup<UnusedButSetParameter>, DefaultIgnore;
def warn_unused_variable : Warning<"unused variable %0">,
InGroup<UnusedVariable>, DefaultIgnore;
+def warn_unused_but_set_variable : Warning<"variable %0 set but not used">,
+ InGroup<UnusedButSetVariable>, DefaultIgnore;
def warn_unused_local_typedef : Warning<
"unused %select{typedef|type alias}0 %1">,
InGroup<UnusedLocalTypedef>, DefaultIgnore;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ 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",
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits