gpanders created this revision.
gpanders added a reviewer: theraven.
Herald added a project: All.
gpanders requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This flag enables warnings for unused function return values
universally, even for functions which do not have a "warn_unused_result"
attribute.
This is disabled by default as it is far too noisy for most C/C++ code;
however, there are contexts where this is a desirable warning to have
(for example, in safety critical code where ignoring return values is
usually forbidden).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D149380
Files:
clang/include/clang/AST/Expr.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/Sema/unused-result-always.c
Index: clang/test/Sema/unused-result-always.c
===================================================================
--- /dev/null
+++ clang/test/Sema/unused-result-always.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-unreachable-code -Wno-strict-prototypes -Wunused-result-always
+
+int t1(int X, int Y);
+void t2();
+void *t3();
+
+#define M1(a, b) t1(a, b)
+
+void bar() {
+ t1(1, 2); // expected-warning {{ignoring return value of function}}
+
+ t2(); // no warning.
+
+ (void)t1(1, 2); // no warning;
+
+ t3(); // expected-warning {{ignoring return value of function}}
+
+ M1(1, 2); // expected-warning {{ignoring return value of function}}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -200,8 +200,13 @@
static bool DiagnoseNoDiscard(Sema &S, const WarnUnusedResultAttr *A,
SourceLocation Loc, SourceRange R1,
SourceRange R2, bool IsCtor) {
- if (!A)
+ if (!A && S.Diags.isIgnored(diag::warn_unused_result_always, Loc))
return false;
+
+ if (!A) {
+ return S.Diag(Loc, diag::warn_unused_result_always) << R1 << R2;
+ }
+
StringRef Msg = A->getMessage();
if (Msg.empty()) {
@@ -242,7 +247,10 @@
const Expr *WarnExpr;
SourceLocation Loc;
SourceRange R1, R2;
- if (!E->isUnusedResultAWarning(WarnExpr, Loc, R1, R2, Context))
+ bool WarnUnusedResultAlways =
+ !Diags.isIgnored(diag::warn_unused_result_always, ExprLoc);
+ if (!E->isUnusedResultAWarning(WarnExpr, Loc, R1, R2, Context,
+ WarnUnusedResultAlways))
return;
// If this is a GNU statement expression expanded from a macro, it is probably
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2578,7 +2578,7 @@
/// warning.
bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc,
SourceRange &R1, SourceRange &R2,
- ASTContext &Ctx) const {
+ ASTContext &Ctx, bool Always) const {
// Don't warn if the expr is type dependent. The type could end up
// instantiating to void.
if (isTypeDependent())
@@ -2593,18 +2593,20 @@
R1 = getSourceRange();
return true;
case ParenExprClass:
- return cast<ParenExpr>(this)->getSubExpr()->
- isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return cast<ParenExpr>(this)->getSubExpr()->isUnusedResultAWarning(
+ WarnE, Loc, R1, R2, Ctx, Always);
case GenericSelectionExprClass:
- return cast<GenericSelectionExpr>(this)->getResultExpr()->
- isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return cast<GenericSelectionExpr>(this)
+ ->getResultExpr()
+ ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx, Always);
case CoawaitExprClass:
case CoyieldExprClass:
- return cast<CoroutineSuspendExpr>(this)->getResumeExpr()->
- isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return cast<CoroutineSuspendExpr>(this)
+ ->getResumeExpr()
+ ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx, Always);
case ChooseExprClass:
- return cast<ChooseExpr>(this)->getChosenSubExpr()->
- isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return cast<ChooseExpr>(this)->getChosenSubExpr()->isUnusedResultAWarning(
+ WarnE, Loc, R1, R2, Ctx, Always);
case UnaryOperatorClass: {
const UnaryOperator *UO = cast<UnaryOperator>(this);
@@ -2632,7 +2634,8 @@
return false;
break;
case UO_Extension:
- return UO->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return UO->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx,
+ Always);
}
WarnE = this;
Loc = UO->getOperatorLoc();
@@ -2653,12 +2656,15 @@
dyn_cast<IntegerLiteral>(BO->getRHS()->IgnoreParens()))
if (IE->getValue() == 0)
return false;
- return BO->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return BO->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx,
+ Always);
// Consider '||', '&&' to have side effects if the LHS or RHS does.
case BO_LAnd:
case BO_LOr:
- if (!BO->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) ||
- !BO->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx))
+ if (!BO->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx,
+ Always) ||
+ !BO->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx,
+ Always))
return false;
break;
}
@@ -2680,12 +2686,15 @@
// be being used for control flow. Only warn if both the LHS and
// RHS are warnings.
const auto *Exp = cast<ConditionalOperator>(this);
- return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) &&
- Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx,
+ Always) &&
+ Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx,
+ Always);
}
case BinaryConditionalOperatorClass: {
const auto *Exp = cast<BinaryConditionalOperator>(this);
- return Exp->getFalseExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return Exp->getFalseExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx,
+ Always);
}
case MemberExprClass:
@@ -2737,13 +2746,14 @@
// If this is a direct call, get the callee.
const CallExpr *CE = cast<CallExpr>(this);
if (const Decl *FD = CE->getCalleeDecl()) {
- // If the callee has attribute pure, const, or warn_unused_result, warn
- // about it. void foo() { strlen("bar"); } should warn.
+ // If the callee has attribute pure, const, or warn_unused_result, or if
+ // -Wunused-result-always is enabled, warn about it.
+ // void foo() { strlen("bar"); } should warn.
//
// Note: If new cases are added here, DiagnoseUnusedExprResult should be
// updated to match for QoI.
- if (CE->hasUnusedResultAttr(Ctx) ||
- FD->hasAttr<PureAttr>() || FD->hasAttr<ConstAttr>()) {
+ if (Always || CE->hasUnusedResultAttr(Ctx) || FD->hasAttr<PureAttr>() ||
+ FD->hasAttr<ConstAttr>()) {
WarnE = this;
Loc = CE->getCallee()->getBeginLoc();
R1 = CE->getCallee()->getSourceRange();
@@ -2845,7 +2855,8 @@
// Otherwise, warn if the result expression would warn.
const Expr *Result = POE->getResultExpr();
- return Result && Result->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return Result &&
+ Result->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx, Always);
}
case StmtExprClass: {
@@ -2857,10 +2868,10 @@
const CompoundStmt *CS = cast<StmtExpr>(this)->getSubStmt();
if (!CS->body_empty()) {
if (const Expr *E = dyn_cast<Expr>(CS->body_back()))
- return E->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return E->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx, Always);
if (const LabelStmt *Label = dyn_cast<LabelStmt>(CS->body_back()))
if (const Expr *E = dyn_cast<Expr>(Label->getSubStmt()))
- return E->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return E->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx, Always);
}
if (getType()->isVoidType())
@@ -2896,7 +2907,7 @@
if (SubE->getType()->isArrayType())
return false;
- return SubE->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return SubE->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx, Always);
}
return false;
}
@@ -2904,7 +2915,8 @@
// If this is a cast to a constructor conversion, check the operand.
// Otherwise, the result of the cast is unused.
if (CE->getCastKind() == CK_ConstructorConversion)
- return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx,
+ Always);
if (CE->getCastKind() == CK_Dependent)
return false;
@@ -2928,14 +2940,15 @@
ICE->getSubExpr()->getType().isVolatileQualified())
return false;
- return ICE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return ICE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx,
+ Always);
}
case CXXDefaultArgExprClass:
- return (cast<CXXDefaultArgExpr>(this)
- ->getExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx));
+ return (cast<CXXDefaultArgExpr>(this)->getExpr()->isUnusedResultAWarning(
+ WarnE, Loc, R1, R2, Ctx, Always));
case CXXDefaultInitExprClass:
- return (cast<CXXDefaultInitExpr>(this)
- ->getExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx));
+ return (cast<CXXDefaultInitExpr>(this)->getExpr()->isUnusedResultAWarning(
+ WarnE, Loc, R1, R2, Ctx, Always));
case CXXNewExprClass:
// FIXME: In theory, there might be new expressions that don't have side
@@ -2945,13 +2958,14 @@
case MaterializeTemporaryExprClass:
return cast<MaterializeTemporaryExpr>(this)
->getSubExpr()
- ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx, Always);
case CXXBindTemporaryExprClass:
- return cast<CXXBindTemporaryExpr>(this)->getSubExpr()
- ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return cast<CXXBindTemporaryExpr>(this)
+ ->getSubExpr()
+ ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx, Always);
case ExprWithCleanupsClass:
- return cast<ExprWithCleanups>(this)->getSubExpr()
- ->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+ return cast<ExprWithCleanups>(this)->getSubExpr()->isUnusedResultAWarning(
+ WarnE, Loc, R1, R2, Ctx, Always);
}
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8790,6 +8790,9 @@
def warn_unused_result_msg : Warning<
"ignoring return value of function declared with %0 attribute: %1">,
InGroup<UnusedResult>;
+def warn_unused_result_always : Warning<
+ "ignoring return value of function">,
+ InGroup<UnusedResultAlways>, DefaultIgnore;
def warn_unused_result_typedef_unsupported_spelling : Warning<
"'[[%select{nodiscard|gnu::warn_unused_result}0]]' attribute ignored when "
"applied to a typedef; consider using '__attribute__((warn_unused_result))' "
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -775,6 +775,7 @@
def UnusedParameter : DiagGroup<"unused-parameter">;
def UnusedButSetParameter : DiagGroup<"unused-but-set-parameter">;
def UnusedResult : DiagGroup<"unused-result">;
+def UnusedResultAlways : DiagGroup<"unused-result-always">;
def PotentiallyEvaluatedExpression : DiagGroup<"potentially-evaluated-expression">;
def UnevaluatedExpression : DiagGroup<"unevaluated-expression",
[PotentiallyEvaluatedExpression]>;
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -254,8 +254,8 @@
/// and ranges with expr to warn on and source locations/ranges appropriate
/// for a warning.
bool isUnusedResultAWarning(const Expr *&WarnExpr, SourceLocation &Loc,
- SourceRange &R1, SourceRange &R2,
- ASTContext &Ctx) const;
+ SourceRange &R1, SourceRange &R2, ASTContext &Ctx,
+ bool Always) const;
/// isLValue - True if this expression is an "l-value" according to
/// the rules of the current language. C and C++ give somewhat
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
