mbenfield updated this revision to Diff 341695.
mbenfield added a comment.
Another try at these warnings, using the implementation strategy outlined by
rsmith.
A couple other notes:
- At the moment I've removed these warnings from the diagnostic groups -Wextra
and -Wunused. It was suggested by aeubanks that the groups could be added in a
later commit, once these warnings have been determined to not be too
disruptive. Of course I can change this now if requested.
- I've just tried to mimic gcc's behavior as closely as possible, including not
warning if an assignment is used, and not warning on nonscalar types in C++
(except that in some cases gcc inexplicably does warn on nonscalar types; test
on the file vector-gcc-compat.c to compare... I haven't determined any
rationale to when it chooses to warn in these 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/SemaExprCXX.cpp
clang/test/Sema/warn-unused-but-set-parameters.c
clang/test/Sema/warn-unused-but-set-variables.c
clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -0,0 +1,41 @@
+// 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;
+ };
+}
+
+void f2() {
+ // Don't warn for either of these cases.
+ constexpr int x = 2;
+ const int y = 1;
+ char a[x];
+ char b[y];
+}
Index: clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/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/warn-unused-but-set-variables.c
===================================================================
--- /dev/null
+++ clang/test/Sema/warn-unused-but-set-variables.c
@@ -0,0 +1,40 @@
+// 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;
+
+ // Don't warn for an extern variable.
+ extern int w;
+ w = 0;
+
+ // Following gcc, this should not warn.
+ int a;
+ w = (a = 0);
+
+ 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-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/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7782,9 +7782,28 @@
return BuildCXXNoexceptExpr(KeyLoc, Operand, RParen);
}
+static void MaybeDecrementCount(
+ Expr *E, llvm::DenseMap<const VarDecl *, int> &RefsMinusAssignments) {
+ BinaryOperator *BO = dyn_cast<BinaryOperator>(E);
+ if (!BO || !BO->isAssignmentOp())
+ return;
+ DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(BO->getLHS());
+ if (!DRE)
+ return;
+ VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl());
+ if (!VD)
+ return;
+ auto iter = RefsMinusAssignments.find(VD);
+ if (iter == RefsMinusAssignments.end())
+ return;
+ iter->getSecond()--;
+}
+
/// Perform the conversions required for an expression used in a
/// context that ignores the result.
ExprResult Sema::IgnoredValueConversions(Expr *E) {
+ MaybeDecrementCount(E, RefsMinusAssignments);
+
if (E->hasPlaceholderType()) {
ExprResult result = CheckPlaceholderExpr(E);
if (result.isInvalid()) return E;
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -18302,8 +18302,9 @@
"MarkVarDeclODRUsed failed to cleanup MaybeODRUseExprs?");
}
-static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc,
- VarDecl *Var, Expr *E) {
+static void DoMarkVarDeclReferenced(
+ Sema &SemaRef, SourceLocation Loc, VarDecl *Var, Expr *E,
+ llvm::DenseMap<const VarDecl *, int> &RefsMinusAssignments) {
assert((!E || isa<DeclRefExpr>(E) || isa<MemberExpr>(E) ||
isa<FunctionParmPackExpr>(E)) &&
"Invalid Expr argument to DoMarkVarDeclReferenced");
@@ -18338,6 +18339,11 @@
bool UsableInConstantExpr =
Var->mightBeUsableInConstantExpressions(SemaRef.Context);
+ if (OdrUse == OdrUseContext::Used && Var->isLocalVarDeclOrParm() &&
+ !Var->hasExternalStorage()) {
+ RefsMinusAssignments.insert({Var, 0}).first->getSecond()++;
+ }
+
// C++20 [expr.const]p12:
// A variable [...] is needed for constant evaluation if it is [...] a
// variable whose name appears as a potentially constant evaluated
@@ -18493,16 +18499,18 @@
/// (C++ [basic.def.odr]p2, C99 6.9p3). Note that this should not be
/// used directly for normal expressions referring to VarDecl.
void Sema::MarkVariableReferenced(SourceLocation Loc, VarDecl *Var) {
- DoMarkVarDeclReferenced(*this, Loc, Var, nullptr);
+ DoMarkVarDeclReferenced(*this, Loc, Var, nullptr, RefsMinusAssignments);
}
-static void MarkExprReferenced(Sema &SemaRef, SourceLocation Loc,
- Decl *D, Expr *E, bool MightBeOdrUse) {
+static void
+MarkExprReferenced(Sema &SemaRef, SourceLocation Loc, Decl *D, Expr *E,
+ bool MightBeOdrUse,
+ llvm::DenseMap<const VarDecl *, int> &RefsMinusAssignments) {
if (SemaRef.isInOpenMPDeclareTargetContext())
SemaRef.checkDeclIsAllowedInOpenMPTarget(E, D);
if (VarDecl *Var = dyn_cast<VarDecl>(D)) {
- DoMarkVarDeclReferenced(SemaRef, Loc, Var, E);
+ DoMarkVarDeclReferenced(SemaRef, Loc, Var, E, RefsMinusAssignments);
return;
}
@@ -18548,7 +18556,8 @@
if (!isConstantEvaluated() && FD->isConsteval() &&
!RebuildingImmediateInvocation)
ExprEvalContexts.back().ReferenceToConsteval.insert(E);
- MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse);
+ MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse,
+ RefsMinusAssignments);
}
/// Perform reference-marking and odr-use handling for a MemberExpr.
@@ -18567,13 +18576,15 @@
}
SourceLocation Loc =
E->getMemberLoc().isValid() ? E->getMemberLoc() : E->getBeginLoc();
- MarkExprReferenced(*this, Loc, E->getMemberDecl(), E, MightBeOdrUse);
+ MarkExprReferenced(*this, Loc, E->getMemberDecl(), E, MightBeOdrUse,
+ RefsMinusAssignments);
}
/// Perform reference-marking and odr-use handling for a FunctionParmPackExpr.
void Sema::MarkFunctionParmPackReferenced(FunctionParmPackExpr *E) {
for (VarDecl *VD : *E)
- MarkExprReferenced(*this, E->getParameterPackLocation(), VD, E, true);
+ MarkExprReferenced(*this, E->getParameterPackLocation(), VD, E, true,
+ RefsMinusAssignments);
}
/// Perform marking for a reference to an arbitrary declaration. It
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1905,6 +1905,28 @@
Diag(D->getLocation(), DiagID) << D << Hint;
}
+void Sema::DiagnoseUnusedButSetDecl(const VarDecl *VD) {
+ // If it's not referenced, it can't be set.
+ if (!VD->isReferenced() || !VD->getDeclName() || VD->hasAttr<UnusedAttr>())
+ return;
+
+ // In C++, don't warn for nonscalar types, which mimics gcc's behavior.
+ if (getLangOpts().CPlusPlus && !VD->getType()->isScalarType())
+ return;
+
+ auto iter = RefsMinusAssignments.find(VD);
+ if (iter == RefsMinusAssignments.end())
+ return;
+
+ assert(iter->getSecond() >= 0 &&
+ "Found a negative number of references to a VarDecl");
+ if (iter->getSecond() != 0)
+ return;
+ unsigned DiagID = isa<ParmVarDecl>(VD) ? diag::warn_unused_but_set_parameter
+ : diag::warn_unused_but_set_variable;
+ Diag(VD->getLocation(), DiagID) << VD;
+}
+
static void CheckPoppedLabel(LabelDecl *L, Sema &S) {
// Verify that we have no forward references left. If so, there was a goto
// or address of a label taken, but no definition of it. Label fwd
@@ -1939,6 +1961,11 @@
DiagnoseUnusedNestedTypedefs(RD);
}
+ if (VarDecl *VD = dyn_cast<VarDecl>(D)) {
+ DiagnoseUnusedButSetDecl(VD);
+ RefsMinusAssignments.erase(VD);
+ }
+
if (!D->getDeclName()) continue;
// If this was a forward reference to a label, verify it was defined.
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1514,6 +1514,11 @@
bool WarnedStackExhausted = false;
+ /// Increment when we find a reference; decrement when we find an ignored
+ /// assignment. Ultimately the value is 0 if every reference is an ignored
+ /// assignment.
+ llvm::DenseMap<const VarDecl *, int> RefsMinusAssignments;
+
public:
Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
TranslationUnitKind TUKind = TU_Complete,
@@ -4856,6 +4861,10 @@
void DiagnoseUnusedNestedTypedefs(const RecordDecl *D);
void DiagnoseUnusedDecl(const NamedDecl *ND);
+ /// If VD is set but not otherwise used, diagnose, for a parameter or a
+ /// variable.
+ void DiagnoseUnusedButSetDecl(const VarDecl *VD);
+
/// Emit \p DiagID if statement located on \p StmtLoc has a suspicious null
/// statement as a \p Body, and it is located on the same line.
///
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
@@ -724,6 +724,7 @@
def UnusedLabel : DiagGroup<"unused-label">;
def UnusedLambdaCapture : DiagGroup<"unused-lambda-capture">;
def UnusedParameter : DiagGroup<"unused-parameter">;
+def UnusedButSetParameter : DiagGroup<"unused-but-set-parameter">;
def UnusedResult : DiagGroup<"unused-result">;
def PotentiallyEvaluatedExpression : DiagGroup<"potentially-evaluated-expression">;
def UnevaluatedExpression : DiagGroup<"unevaluated-expression",
@@ -733,6 +734,7 @@
def UnusedConstVariable : DiagGroup<"unused-const-variable">;
def UnusedVariable : DiagGroup<"unused-variable",
[UnusedConstVariable]>;
+def UnusedButSetVariable : DiagGroup<"unused-but-set-variable">;
def UnusedLocalTypedef : DiagGroup<"unused-local-typedef">;
def UnusedPropertyIvar : DiagGroup<"unused-property-ivar">;
def UnusedGetterReturnValue : DiagGroup<"unused-getter-return-value">;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
