Refactor the way we handle diagnosing unused expression results.
Rather than sprinkle calls to DiagnoseUnusedExprResult() around in places where we want diagnostics, we now diagnose unused expression statements and full expressions in a more generic way when acting on the final expression statement. This results in more appropriate diagnostics for [[nodiscard]] where we were previously lacking them, such as when the body of a for loop is not a compound statement.
This patch fixes PR39837.
llvm-svn: 350404
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 46e4431..438ff0e 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -360,6 +360,11 @@
/// just a regular sub-expression.
SourceLocation ExprStatementTokLoc;
+ /// Tests whether an expression value is discarded based on token lookahead.
+ /// It will return true if the lexer is currently processing the })
+ /// terminating a GNU statement expression and false otherwise.
+ bool isExprValueDiscarded();
+
public:
Parser(Preprocessor &PP, Sema &Actions, bool SkipFunctionBodies);
~Parser() override;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 6d002b3..92bebca 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1365,6 +1365,7 @@
void PopCompoundScope();
sema::CompoundScopeInfo &getCurCompoundScope() const;
+ bool isCurCompoundStmtAStmtExpr() const;
bool hasAnyUnrecoverableErrorsInThisFunction() const;
@@ -3685,16 +3686,17 @@
return MakeFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation());
}
FullExprArg MakeFullExpr(Expr *Arg, SourceLocation CC) {
- return FullExprArg(ActOnFinishFullExpr(Arg, CC).get());
+ return FullExprArg(
+ ActOnFinishFullExpr(Arg, CC, /*DiscardedValue*/ false).get());
}
FullExprArg MakeFullDiscardedValueExpr(Expr *Arg) {
ExprResult FE =
- ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation(),
- /*DiscardedValue*/ true);
+ ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation(),
+ /*DiscardedValue*/ true);
return FullExprArg(FE.get());
}
- StmtResult ActOnExprStmt(ExprResult Arg);
+ StmtResult ActOnExprStmt(ExprResult Arg, bool DiscardedValue = true);
StmtResult ActOnExprStmtError();
StmtResult ActOnNullStmt(SourceLocation SemiLoc,
@@ -5340,13 +5342,12 @@
CreateMaterializeTemporaryExpr(QualType T, Expr *Temporary,
bool BoundToLvalueReference);
- ExprResult ActOnFinishFullExpr(Expr *Expr) {
- return ActOnFinishFullExpr(Expr, Expr ? Expr->getExprLoc()
- : SourceLocation());
+ ExprResult ActOnFinishFullExpr(Expr *Expr, bool DiscardedValue) {
+ return ActOnFinishFullExpr(
+ Expr, Expr ? Expr->getExprLoc() : SourceLocation(), DiscardedValue);
}
ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
- bool DiscardedValue = false,
- bool IsConstexpr = false);
+ bool DiscardedValue, bool IsConstexpr = false);
StmtResult ActOnFinishFullStmt(Stmt *Stmt);
// Marks SS invalid if it represents an incomplete type.
diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index c8d7bda..bd55f71 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -2741,7 +2741,7 @@
// Otherwise, eat the semicolon.
ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
- return Actions.ActOnExprStmt(Res);
+ return Actions.ActOnExprStmt(Res, isExprValueDiscarded());
}
ExprResult Parser::ParseObjCAtExpression(SourceLocation AtLoc) {
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 17c3fa3..dd2a8aa 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -314,7 +314,7 @@
Actions.ActOnOpenMPDeclareReductionCombinerStart(getCurScope(), D);
ExprResult CombinerResult =
Actions.ActOnFinishFullExpr(ParseAssignmentExpression().get(),
- D->getLocation(), /*DiscardedValue=*/true);
+ D->getLocation(), /*DiscardedValue*/ false);
Actions.ActOnOpenMPDeclareReductionCombinerEnd(D, CombinerResult.get());
if (CombinerResult.isInvalid() && Tok.isNot(tok::r_paren) &&
@@ -356,7 +356,7 @@
if (Actions.getLangOpts().CPlusPlus) {
InitializerResult = Actions.ActOnFinishFullExpr(
ParseAssignmentExpression().get(), D->getLocation(),
- /*DiscardedValue=*/true);
+ /*DiscardedValue*/ false);
} else {
ConsumeToken();
ParseOpenMPReductionInitializerForDecl(OmpPrivParm);
@@ -364,7 +364,7 @@
} else {
InitializerResult = Actions.ActOnFinishFullExpr(
ParseAssignmentExpression().get(), D->getLocation(),
- /*DiscardedValue=*/true);
+ /*DiscardedValue*/ false);
}
Actions.ActOnOpenMPDeclareReductionInitializerEnd(
D, InitializerResult.get(), OmpPrivParm);
@@ -1455,7 +1455,7 @@
ExprResult LHS(ParseCastExpression(
/*isUnaryExpression=*/false, /*isAddressOfOperand=*/false, NotTypeCast));
ExprResult Val(ParseRHSOfBinaryExpression(LHS, prec::Conditional));
- Val = Actions.ActOnFinishFullExpr(Val.get(), ELoc);
+ Val = Actions.ActOnFinishFullExpr(Val.get(), ELoc, /*DiscardedValue*/ false);
// Parse ')'.
RLoc = Tok.getLocation();
@@ -1711,7 +1711,8 @@
SourceLocation ELoc = Tok.getLocation();
ExprResult LHS(ParseCastExpression(false, false, NotTypeCast));
Val = ParseRHSOfBinaryExpression(LHS, prec::Conditional);
- Val = Actions.ActOnFinishFullExpr(Val.get(), ELoc);
+ Val =
+ Actions.ActOnFinishFullExpr(Val.get(), ELoc, /*DiscardedValue*/ false);
}
// Parse ')'.
@@ -1996,7 +1997,8 @@
Data.ColonLoc = Tok.getLocation();
SourceLocation ELoc = ConsumeToken();
ExprResult Tail = ParseAssignmentExpression();
- Tail = Actions.ActOnFinishFullExpr(Tail.get(), ELoc);
+ Tail =
+ Actions.ActOnFinishFullExpr(Tail.get(), ELoc, /*DiscardedValue*/ false);
if (Tail.isUsable())
Data.TailExpr = Tail.get();
else
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 313793c..2974e6a 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -439,7 +439,7 @@
// Otherwise, eat the semicolon.
ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
- return Actions.ActOnExprStmt(Expr);
+ return Actions.ActOnExprStmt(Expr, isExprValueDiscarded());
}
/// ParseSEHTryBlockCommon
@@ -958,6 +958,16 @@
return true;
}
+bool Parser::isExprValueDiscarded() {
+ if (Actions.isCurCompoundStmtAStmtExpr()) {
+ // Look to see if the next two tokens close the statement expression;
+ // if so, this expression statement is the last statement in a
+ // statment expression.
+ return Tok.isNot(tok::r_brace) || NextToken().isNot(tok::r_paren);
+ }
+ return true;
+}
+
/// ParseCompoundStatementBody - Parse a sequence of statements and invoke the
/// ActOnCompoundStmt action. This expects the '{' to be the current token, and
/// consume the '}' at the end of the block. It does not manipulate the scope
@@ -1062,7 +1072,7 @@
// Eat the semicolon at the end of stmt and convert the expr into a
// statement.
ExpectAndConsumeSemi(diag::err_expected_semi_after_expr);
- R = Actions.ActOnExprStmt(Res);
+ R = Actions.ActOnExprStmt(Res, isExprValueDiscarded());
}
}
@@ -1698,8 +1708,16 @@
if (!Value.isInvalid()) {
if (ForEach)
FirstPart = Actions.ActOnForEachLValueExpr(Value.get());
- else
- FirstPart = Actions.ActOnExprStmt(Value);
+ else {
+ // We already know this is not an init-statement within a for loop, so
+ // if we are parsing a C++11 range-based for loop, we should treat this
+ // expression statement as being a discarded value expression because
+ // we will err below. This way we do not warn on an unused expression
+ // that was an error in the first place, like with: for (expr : expr);
+ bool IsRangeBasedFor =
+ getLangOpts().CPlusPlus11 && !ForEach && Tok.is(tok::colon);
+ FirstPart = Actions.ActOnExprStmt(Value, !IsRangeBasedFor);
+ }
}
if (Tok.is(tok::semi)) {
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index cc79238..a84116c1 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -647,7 +647,7 @@
return StmtError();
Suspend = BuildResolvedCoawaitExpr(Loc, Suspend.get(),
/*IsImplicit*/ true);
- Suspend = ActOnFinishFullExpr(Suspend.get());
+ Suspend = ActOnFinishFullExpr(Suspend.get(), /*DiscardedValue*/ false);
if (Suspend.isInvalid()) {
Diag(Loc, diag::note_coroutine_promise_suspend_implicitly_required)
<< ((Name == "initial_suspend") ? 0 : 1);
@@ -868,7 +868,7 @@
if (PC.isInvalid())
return StmtError();
- Expr *PCE = ActOnFinishFullExpr(PC.get()).get();
+ Expr *PCE = ActOnFinishFullExpr(PC.get(), /*DiscardedValue*/ false).get();
Stmt *Res = new (Context) CoreturnStmt(Loc, E, PCE, IsImplicit);
return Res;
@@ -1237,7 +1237,7 @@
ExprResult NewExpr =
S.ActOnCallExpr(S.getCurScope(), NewRef.get(), Loc, NewArgs, Loc);
- NewExpr = S.ActOnFinishFullExpr(NewExpr.get());
+ NewExpr = S.ActOnFinishFullExpr(NewExpr.get(), /*DiscardedValue*/ false);
if (NewExpr.isInvalid())
return false;
@@ -1263,7 +1263,8 @@
ExprResult DeleteExpr =
S.ActOnCallExpr(S.getCurScope(), DeleteRef.get(), Loc, DeleteArgs, Loc);
- DeleteExpr = S.ActOnFinishFullExpr(DeleteExpr.get());
+ DeleteExpr =
+ S.ActOnFinishFullExpr(DeleteExpr.get(), /*DiscardedValue*/ false);
if (DeleteExpr.isInvalid())
return false;
@@ -1348,7 +1349,8 @@
ExprResult UnhandledException = buildPromiseCall(S, Fn.CoroutinePromise, Loc,
"unhandled_exception", None);
- UnhandledException = S.ActOnFinishFullExpr(UnhandledException.get(), Loc);
+ UnhandledException = S.ActOnFinishFullExpr(UnhandledException.get(), Loc,
+ /*DiscardedValue*/ false);
if (UnhandledException.isInvalid())
return false;
@@ -1401,7 +1403,8 @@
"get_return_object type must no longer be dependent");
if (FnRetType->isVoidType()) {
- ExprResult Res = S.ActOnFinishFullExpr(this->ReturnValue, Loc);
+ ExprResult Res =
+ S.ActOnFinishFullExpr(this->ReturnValue, Loc, /*DiscardedValue*/ false);
if (Res.isInvalid())
return false;
@@ -1433,7 +1436,7 @@
if (Res.isInvalid())
return false;
- Res = S.ActOnFinishFullExpr(Res.get());
+ Res = S.ActOnFinishFullExpr(Res.get(), /*DiscardedValue*/ false);
if (Res.isInvalid())
return false;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 80cafbe..40b0ed3 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11204,9 +11204,9 @@
// struct T { S a, b; } t = { Temp(), Temp() }
//
// we should destroy the first Temp before constructing the second.
- ExprResult Result = ActOnFinishFullExpr(Init, VDecl->getLocation(),
- false,
- VDecl->isConstexpr());
+ ExprResult Result =
+ ActOnFinishFullExpr(Init, VDecl->getLocation(),
+ /*DiscardedValue*/ false, VDecl->isConstexpr());
if (Result.isInvalid()) {
VDecl->setInvalidDecl();
return;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8973d63..2b380bf 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1205,7 +1205,7 @@
E = Seq.Perform(S, Entity, Kind, Init);
if (E.isInvalid())
return true;
- E = S.ActOnFinishFullExpr(E.get(), Loc);
+ E = S.ActOnFinishFullExpr(E.get(), Loc, /*DiscardedValue*/ false);
if (E.isInvalid())
return true;
RefVD->setInit(E.get());
@@ -3682,7 +3682,7 @@
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
- Init = ActOnFinishFullExpr(Init.get(), InitLoc);
+ Init = ActOnFinishFullExpr(Init.get(), InitLoc, /*DiscardedValue*/ false);
if (Init.isInvalid()) {
FD->setInvalidDecl();
return;
@@ -4040,7 +4040,8 @@
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
- MemberInit = ActOnFinishFullExpr(MemberInit.get(), InitRange.getBegin());
+ MemberInit = ActOnFinishFullExpr(MemberInit.get(), InitRange.getBegin(),
+ /*DiscardedValue*/ false);
if (MemberInit.isInvalid())
return true;
@@ -4095,8 +4096,8 @@
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
- DelegationInit = ActOnFinishFullExpr(DelegationInit.get(),
- InitRange.getBegin());
+ DelegationInit = ActOnFinishFullExpr(
+ DelegationInit.get(), InitRange.getBegin(), /*DiscardedValue*/ false);
if (DelegationInit.isInvalid())
return true;
@@ -4225,7 +4226,8 @@
// C++11 [class.base.init]p7:
// The initialization of each base and member constitutes a
// full-expression.
- BaseInit = ActOnFinishFullExpr(BaseInit.get(), InitRange.getBegin());
+ BaseInit = ActOnFinishFullExpr(BaseInit.get(), InitRange.getBegin(),
+ /*DiscardedValue*/ false);
if (BaseInit.isInvalid())
return true;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index f8b991b..aa4b23b 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4736,8 +4736,9 @@
if (Result.isInvalid())
return true;
- Result = ActOnFinishFullExpr(Result.getAs<Expr>(),
- Param->getOuterLocStart());
+ Result =
+ ActOnFinishFullExpr(Result.getAs<Expr>(), Param->getOuterLocStart(),
+ /*DiscardedValue*/ false);
if (Result.isInvalid())
return true;
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 730c426..1c210d3 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -7803,6 +7803,8 @@
FullExpr = IgnoredValueConversions(FullExpr.get());
if (FullExpr.isInvalid())
return ExprError();
+
+ DiagnoseUnusedExprResult(FullExpr.get());
}
FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index 6dc93d0..c1c4572 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -1724,7 +1724,7 @@
/*NRVO=*/false),
CurrentLocation, Src);
if (!Init.isInvalid())
- Init = ActOnFinishFullExpr(Init.get());
+ Init = ActOnFinishFullExpr(Init.get(), /*DiscardedValue*/ false);
if (Init.isInvalid())
return ExprError();
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 78bef59..59c4b49 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -5320,7 +5320,7 @@
LastIteration.get(), UB.get());
EUB = SemaRef.BuildBinOp(CurScope, InitLoc, BO_Assign, UB.get(),
CondOp.get());
- EUB = SemaRef.ActOnFinishFullExpr(EUB.get());
+ EUB = SemaRef.ActOnFinishFullExpr(EUB.get(), /*DiscardedValue*/ false);
// If we have a combined directive that combines 'distribute', 'for' or
// 'simd' we need to be able to access the bounds of the schedule of the
@@ -5349,7 +5349,8 @@
LastIteration.get(), CombUB.get());
CombEUB = SemaRef.BuildBinOp(CurScope, InitLoc, BO_Assign, CombUB.get(),
CombCondOp.get());
- CombEUB = SemaRef.ActOnFinishFullExpr(CombEUB.get());
+ CombEUB =
+ SemaRef.ActOnFinishFullExpr(CombEUB.get(), /*DiscardedValue*/ false);
const CapturedDecl *CD = cast<CapturedStmt>(AStmt)->getCapturedDecl();
// We expect to have at least 2 more parameters than the 'parallel'
@@ -5383,7 +5384,7 @@
? LB.get()
: SemaRef.ActOnIntegerConstant(SourceLocation(), 0).get();
Init = SemaRef.BuildBinOp(CurScope, InitLoc, BO_Assign, IV.get(), RHS);
- Init = SemaRef.ActOnFinishFullExpr(Init.get());
+ Init = SemaRef.ActOnFinishFullExpr(Init.get(), /*DiscardedValue*/ false);
if (isOpenMPLoopBoundSharingDirective(DKind)) {
Expr *CombRHS =
@@ -5394,7 +5395,8 @@
: SemaRef.ActOnIntegerConstant(SourceLocation(), 0).get();
CombInit =
SemaRef.BuildBinOp(CurScope, InitLoc, BO_Assign, IV.get(), CombRHS);
- CombInit = SemaRef.ActOnFinishFullExpr(CombInit.get());
+ CombInit =
+ SemaRef.ActOnFinishFullExpr(CombInit.get(), /*DiscardedValue*/ false);
}
}
@@ -5426,7 +5428,7 @@
if (!Inc.isUsable())
return 0;
Inc = SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, IV.get(), Inc.get());
- Inc = SemaRef.ActOnFinishFullExpr(Inc.get());
+ Inc = SemaRef.ActOnFinishFullExpr(Inc.get(), /*DiscardedValue*/ false);
if (!Inc.isUsable())
return 0;
@@ -5444,7 +5446,8 @@
// LB = LB + ST
NextLB =
SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, LB.get(), NextLB.get());
- NextLB = SemaRef.ActOnFinishFullExpr(NextLB.get());
+ NextLB =
+ SemaRef.ActOnFinishFullExpr(NextLB.get(), /*DiscardedValue*/ false);
if (!NextLB.isUsable())
return 0;
// UB + ST
@@ -5454,7 +5457,8 @@
// UB = UB + ST
NextUB =
SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, UB.get(), NextUB.get());
- NextUB = SemaRef.ActOnFinishFullExpr(NextUB.get());
+ NextUB =
+ SemaRef.ActOnFinishFullExpr(NextUB.get(), /*DiscardedValue*/ false);
if (!NextUB.isUsable())
return 0;
if (isOpenMPLoopBoundSharingDirective(DKind)) {
@@ -5465,7 +5469,8 @@
// LB = LB + ST
CombNextLB = SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, CombLB.get(),
CombNextLB.get());
- CombNextLB = SemaRef.ActOnFinishFullExpr(CombNextLB.get());
+ CombNextLB = SemaRef.ActOnFinishFullExpr(CombNextLB.get(),
+ /*DiscardedValue*/ false);
if (!CombNextLB.isUsable())
return 0;
// UB + ST
@@ -5476,7 +5481,8 @@
// UB = UB + ST
CombNextUB = SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, CombUB.get(),
CombNextUB.get());
- CombNextUB = SemaRef.ActOnFinishFullExpr(CombNextUB.get());
+ CombNextUB = SemaRef.ActOnFinishFullExpr(CombNextUB.get(),
+ /*DiscardedValue*/ false);
if (!CombNextUB.isUsable())
return 0;
}
@@ -5497,7 +5503,8 @@
assert(DistInc.isUsable() && "distribute inc expr was not built");
DistInc = SemaRef.BuildBinOp(CurScope, DistIncLoc, BO_Assign, IV.get(),
DistInc.get());
- DistInc = SemaRef.ActOnFinishFullExpr(DistInc.get());
+ DistInc =
+ SemaRef.ActOnFinishFullExpr(DistInc.get(), /*DiscardedValue*/ false);
assert(DistInc.isUsable() && "distribute inc expr was not built");
// Build expression: UB = min(UB, prevUB) for #for in composite or combined
@@ -5509,7 +5516,8 @@
DistEUBLoc, DistEUBLoc, IsUBGreater.get(), PrevUB.get(), UB.get());
PrevEUB = SemaRef.BuildBinOp(CurScope, DistIncLoc, BO_Assign, UB.get(),
CondOp.get());
- PrevEUB = SemaRef.ActOnFinishFullExpr(PrevEUB.get());
+ PrevEUB =
+ SemaRef.ActOnFinishFullExpr(PrevEUB.get(), /*DiscardedValue*/ false);
// Build IV <= PrevUB to be used in parallel for is in combination with
// a distribute directive with schedule(static, 1)
@@ -5613,8 +5621,10 @@
Built.IterationVarRef = IV.get();
Built.LastIteration = LastIteration.get();
Built.NumIterations = NumIterations.get();
- Built.CalcLastIteration =
- SemaRef.ActOnFinishFullExpr(CalcLastIteration.get()).get();
+ Built.CalcLastIteration = SemaRef
+ .ActOnFinishFullExpr(CalcLastIteration.get(),
+ /*DiscardedValue*/ false)
+ .get();
Built.PreCond = PreCond.get();
Built.PreInits = buildPreInits(C, Captures);
Built.Cond = Cond.get();
@@ -10267,8 +10277,8 @@
PseudoDstExpr, PseudoSrcExpr);
if (AssignmentOp.isInvalid())
continue;
- AssignmentOp = ActOnFinishFullExpr(AssignmentOp.get(), ELoc,
- /*DiscardedValue=*/true);
+ AssignmentOp =
+ ActOnFinishFullExpr(AssignmentOp.get(), ELoc, /*DiscardedValue*/ false);
if (AssignmentOp.isInvalid())
continue;
@@ -11274,7 +11284,8 @@
BO_Assign, LHSDRE, ConditionalOp);
}
if (ReductionOp.isUsable())
- ReductionOp = S.ActOnFinishFullExpr(ReductionOp.get());
+ ReductionOp = S.ActOnFinishFullExpr(ReductionOp.get(),
+ /*DiscardedValue*/ false);
}
if (!ReductionOp.isUsable())
continue;
@@ -11612,7 +11623,7 @@
buildDeclRefExpr(*this, SaveVar, StepExpr->getType(), StepLoc);
ExprResult CalcStep =
BuildBinOp(CurScope, StepLoc, BO_Assign, SaveRef.get(), StepExpr);
- CalcStep = ActOnFinishFullExpr(CalcStep.get());
+ CalcStep = ActOnFinishFullExpr(CalcStep.get(), /*DiscardedValue*/ false);
// Warn about zero linear step (it would be probably better specified as
// making corresponding variables 'const').
@@ -11700,7 +11711,7 @@
else
Update = *CurPrivate;
Update = SemaRef.ActOnFinishFullExpr(Update.get(), DE->getBeginLoc(),
- /*DiscardedValue=*/true);
+ /*DiscardedValue*/ false);
// Build final: Var = InitExpr + NumIterations * Step
ExprResult Final;
@@ -11711,7 +11722,7 @@
else
Final = *CurPrivate;
Final = SemaRef.ActOnFinishFullExpr(Final.get(), DE->getBeginLoc(),
- /*DiscardedValue=*/true);
+ /*DiscardedValue*/ false);
if (!Update.isUsable() || !Final.isUsable()) {
Updates.push_back(nullptr);
@@ -11879,7 +11890,7 @@
if (AssignmentOp.isInvalid())
continue;
AssignmentOp = ActOnFinishFullExpr(AssignmentOp.get(), DE->getExprLoc(),
- /*DiscardedValue=*/true);
+ /*DiscardedValue*/ false);
if (AssignmentOp.isInvalid())
continue;
@@ -11987,8 +11998,8 @@
DSAStack->getCurScope(), ELoc, BO_Assign, PseudoDstExpr, PseudoSrcExpr);
if (AssignmentOp.isInvalid())
continue;
- AssignmentOp = ActOnFinishFullExpr(AssignmentOp.get(), ELoc,
- /*DiscardedValue=*/true);
+ AssignmentOp =
+ ActOnFinishFullExpr(AssignmentOp.get(), ELoc, /*DiscardedValue*/ false);
if (AssignmentOp.isInvalid())
continue;
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index dacf8d0..9e30c9a3 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -42,12 +42,11 @@
using namespace clang;
using namespace sema;
-StmtResult Sema::ActOnExprStmt(ExprResult FE) {
+StmtResult Sema::ActOnExprStmt(ExprResult FE, bool DiscardedValue) {
if (FE.isInvalid())
return StmtError();
- FE = ActOnFinishFullExpr(FE.get(), FE.get()->getExprLoc(),
- /*DiscardedValue*/ true);
+ FE = ActOnFinishFullExpr(FE.get(), FE.get()->getExprLoc(), DiscardedValue);
if (FE.isInvalid())
return StmtError();
@@ -348,6 +347,10 @@
return getCurFunction()->CompoundScopes.back();
}
+bool Sema::isCurCompoundStmtAStmtExpr() const {
+ return getCurCompoundScope().IsStmtExpr;
+}
+
StmtResult Sema::ActOnCompoundStmt(SourceLocation L, SourceLocation R,
ArrayRef<Stmt *> Elts, bool isStmtExpr) {
const unsigned NumElts = Elts.size();
@@ -370,14 +373,6 @@
Diag(D->getLocation(), diag::ext_mixed_decls_code);
}
}
- // Warn about unused expressions in statements.
- for (unsigned i = 0; i != NumElts; ++i) {
- // Ignore statements that are last in a statement expression.
- if (isStmtExpr && i == NumElts - 1)
- continue;
-
- DiagnoseUnusedExprResult(Elts[i]);
- }
// Check for suspicious empty body (null statement) in `for' and `while'
// statements. Don't do anything for template instantiations, this just adds
@@ -469,15 +464,12 @@
/// ActOnCaseStmtBody - This installs a statement as the body of a case.
void Sema::ActOnCaseStmtBody(Stmt *S, Stmt *SubStmt) {
- DiagnoseUnusedExprResult(SubStmt);
cast<CaseStmt>(S)->setSubStmt(SubStmt);
}
StmtResult
Sema::ActOnDefaultStmt(SourceLocation DefaultLoc, SourceLocation ColonLoc,
Stmt *SubStmt, Scope *CurScope) {
- DiagnoseUnusedExprResult(SubStmt);
-
if (getCurFunction()->SwitchStack.empty()) {
Diag(DefaultLoc, diag::err_default_not_in_switch);
return SubStmt;
@@ -571,9 +563,6 @@
if (IsConstexpr || isa<ObjCAvailabilityCheckExpr>(Cond.get().second))
setFunctionHasBranchProtectedScope();
- DiagnoseUnusedExprResult(thenStmt);
- DiagnoseUnusedExprResult(elseStmt);
-
return IfStmt::Create(Context, IfLoc, IsConstexpr, InitStmt, Cond.get().first,
Cond.get().second, thenStmt, ElseLoc, elseStmt);
}
@@ -1301,8 +1290,6 @@
!Diags.isIgnored(diag::warn_comma_operator, CondVal.second->getExprLoc()))
CommaVisitor(*this).Visit(CondVal.second);
- DiagnoseUnusedExprResult(Body);
-
if (isa<NullStmt>(Body))
getCurCompoundScope().setHasEmptyLoopBodies();
@@ -1322,7 +1309,7 @@
return StmtError();
Cond = CondResult.get();
- CondResult = ActOnFinishFullExpr(Cond, DoLoc);
+ CondResult = ActOnFinishFullExpr(Cond, DoLoc, /*DiscardedValue*/ false);
if (CondResult.isInvalid())
return StmtError();
Cond = CondResult.get();
@@ -1332,8 +1319,6 @@
!Diags.isIgnored(diag::warn_comma_operator, Cond->getExprLoc()))
CommaVisitor(*this).Visit(Cond);
- DiagnoseUnusedExprResult(Body);
-
return new (Context) DoStmt(Body, Cond, DoLoc, WhileLoc, CondRParen);
}
@@ -1778,11 +1763,6 @@
CommaVisitor(*this).Visit(Second.get().second);
Expr *Third = third.release().getAs<Expr>();
-
- DiagnoseUnusedExprResult(First);
- DiagnoseUnusedExprResult(Third);
- DiagnoseUnusedExprResult(Body);
-
if (isa<NullStmt>(Body))
getCurCompoundScope().setHasEmptyLoopBodies();
@@ -1802,7 +1782,7 @@
if (result.isInvalid()) return StmtError();
E = result.get();
- ExprResult FullExpr = ActOnFinishFullExpr(E);
+ ExprResult FullExpr = ActOnFinishFullExpr(E, /*DiscardedValue*/ false);
if (FullExpr.isInvalid())
return StmtError();
return StmtResult(static_cast<Stmt*>(FullExpr.get()));
@@ -1956,7 +1936,8 @@
if (CollectionExprResult.isInvalid())
return StmtError();
- CollectionExprResult = ActOnFinishFullExpr(CollectionExprResult.get());
+ CollectionExprResult =
+ ActOnFinishFullExpr(CollectionExprResult.get(), /*DiscardedValue*/ false);
if (CollectionExprResult.isInvalid())
return StmtError();
@@ -2593,7 +2574,8 @@
if (!NotEqExpr.isInvalid())
NotEqExpr = CheckBooleanCondition(ColonLoc, NotEqExpr.get());
if (!NotEqExpr.isInvalid())
- NotEqExpr = ActOnFinishFullExpr(NotEqExpr.get());
+ NotEqExpr =
+ ActOnFinishFullExpr(NotEqExpr.get(), /*DiscardedValue*/ false);
if (NotEqExpr.isInvalid()) {
Diag(RangeLoc, diag::note_for_range_invalid_iterator)
<< RangeLoc << 0 << BeginRangeRef.get()->getType();
@@ -2616,7 +2598,7 @@
// co_await during the initial parse.
IncrExpr = ActOnCoawaitExpr(S, CoawaitLoc, IncrExpr.get());
if (!IncrExpr.isInvalid())
- IncrExpr = ActOnFinishFullExpr(IncrExpr.get());
+ IncrExpr = ActOnFinishFullExpr(IncrExpr.get(), /*DiscardedValue*/ false);
if (IncrExpr.isInvalid()) {
Diag(RangeLoc, diag::note_for_range_invalid_iterator)
<< RangeLoc << 2 << BeginRangeRef.get()->getType() ;
@@ -2871,7 +2853,7 @@
return StmtError();
}
- ExprResult ExprRes = ActOnFinishFullExpr(E);
+ ExprResult ExprRes = ActOnFinishFullExpr(E, /*DiscardedValue*/ false);
if (ExprRes.isInvalid())
return StmtError();
E = ExprRes.get();
@@ -3221,7 +3203,8 @@
ExpressionEvaluationContext::DiscardedStatement &&
(HasDeducedReturnType || CurCap->HasImplicitReturnType)) {
if (RetValExp) {
- ExprResult ER = ActOnFinishFullExpr(RetValExp, ReturnLoc);
+ ExprResult ER =
+ ActOnFinishFullExpr(RetValExp, ReturnLoc, /*DiscardedValue*/ false);
if (ER.isInvalid())
return StmtError();
RetValExp = ER.get();
@@ -3348,7 +3331,8 @@
}
if (RetValExp) {
- ExprResult ER = ActOnFinishFullExpr(RetValExp, ReturnLoc);
+ ExprResult ER =
+ ActOnFinishFullExpr(RetValExp, ReturnLoc, /*DiscardedValue*/ false);
if (ER.isInvalid())
return StmtError();
RetValExp = ER.get();
@@ -3578,7 +3562,8 @@
ExpressionEvaluationContext::DiscardedStatement &&
FnRetType->getContainedAutoType()) {
if (RetValExp) {
- ExprResult ER = ActOnFinishFullExpr(RetValExp, ReturnLoc);
+ ExprResult ER =
+ ActOnFinishFullExpr(RetValExp, ReturnLoc, /*DiscardedValue*/ false);
if (ER.isInvalid())
return StmtError();
RetValExp = ER.get();
@@ -3672,7 +3657,8 @@
}
if (RetValExp) {
- ExprResult ER = ActOnFinishFullExpr(RetValExp, ReturnLoc);
+ ExprResult ER =
+ ActOnFinishFullExpr(RetValExp, ReturnLoc, /*DiscardedValue*/ false);
if (ER.isInvalid())
return StmtError();
RetValExp = ER.get();
@@ -3751,7 +3737,8 @@
}
if (RetValExp) {
- ExprResult ER = ActOnFinishFullExpr(RetValExp, ReturnLoc);
+ ExprResult ER =
+ ActOnFinishFullExpr(RetValExp, ReturnLoc, /*DiscardedValue*/ false);
if (ER.isInvalid())
return StmtError();
RetValExp = ER.get();
@@ -3804,7 +3791,7 @@
if (Result.isInvalid())
return StmtError();
- Result = ActOnFinishFullExpr(Result.get());
+ Result = ActOnFinishFullExpr(Result.get(), /*DiscardedValue*/ false);
if (Result.isInvalid())
return StmtError();
Throw = Result.get();
@@ -3876,7 +3863,7 @@
}
// The operand to @synchronized is a full-expression.
- return ActOnFinishFullExpr(operand);
+ return ActOnFinishFullExpr(operand, /*DiscardedValue*/ false);
}
StmtResult
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 9de4e8d..df14768 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -328,7 +328,7 @@
/// other mechanism.
///
/// \returns the transformed statement.
- StmtResult TransformStmt(Stmt *S);
+ StmtResult TransformStmt(Stmt *S, bool DiscardedValue = false);
/// Transform the given statement.
///
@@ -3269,8 +3269,8 @@
bool DeducibleTSTContext);
};
-template<typename Derived>
-StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S) {
+template <typename Derived>
+StmtResult TreeTransform<Derived>::TransformStmt(Stmt *S, bool DiscardedValue) {
if (!S)
return S;
@@ -3294,7 +3294,7 @@
if (E.isInvalid())
return StmtError();
- return getSema().ActOnExprStmt(E);
+ return getSema().ActOnExprStmt(E, DiscardedValue);
}
}
@@ -4715,7 +4715,8 @@
}
if (SizeResult.isInvalid())
return QualType();
- SizeResult = SemaRef.ActOnFinishFullExpr(SizeResult.get());
+ SizeResult =
+ SemaRef.ActOnFinishFullExpr(SizeResult.get(), /*DiscardedValue*/ false);
if (SizeResult.isInvalid())
return QualType();
@@ -6520,7 +6521,9 @@
bool SubStmtChanged = false;
SmallVector<Stmt*, 8> Statements;
for (auto *B : S->body()) {
- StmtResult Result = getDerived().TransformStmt(B);
+ StmtResult Result =
+ getDerived().TransformStmt(B, !IsStmtExpr || B != S->body_back());
+
if (Result.isInvalid()) {
// Immediately fail if this was a DeclStmt, since it's very
// likely that this will cause problems for future statements.
diff --git a/clang/test/CXX/stmt.stmt/stmt.select/p3.cpp b/clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
index 7a6a408..4804cc5 100644
--- a/clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ b/clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -std=c++1z -Wc++14-compat -verify %s -DCPP17
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -std=c++1z -Wc++14-compat -verify %s -DCPP17
int f();
@@ -71,7 +71,6 @@
// last loop above. It would be nice to remove this.
void whileInitStatement2() {
while (; false) {} // expected-error {{expected expression}}
- // expected-warning@-1 {{expression result unused}}
- // expected-error@-2 {{expected ';' after expression}}
- // expected-error@-3 {{expected expression}}
+ // expected-error@-1 {{expected ';' after expression}}
+ // expected-error@-2 {{expected expression}}
}
diff --git a/clang/test/CodeCompletion/pragma-macro-token-caching.c b/clang/test/CodeCompletion/pragma-macro-token-caching.c
index 432706e..59b6621 100644
--- a/clang/test/CodeCompletion/pragma-macro-token-caching.c
+++ b/clang/test/CodeCompletion/pragma-macro-token-caching.c
@@ -12,7 +12,7 @@
void completeParamPragmaError(int param) {
Outer(__extension__({ _Pragma(2) })); // expected-error {{_Pragma takes a parenthesized string literal}}
- param;
+ param; // expected-warning {{expression result unused}}
}
// RUN: %clang_cc1 -fsyntax-only -verify -code-completion-at=%s:16:1 %s | FileCheck %s
diff --git a/clang/test/Parser/cxx1z-init-statement.cpp b/clang/test/Parser/cxx1z-init-statement.cpp
index 3d119ef..ade60dc 100644
--- a/clang/test/Parser/cxx1z-init-statement.cpp
+++ b/clang/test/Parser/cxx1z-init-statement.cpp
@@ -13,9 +13,9 @@
if (T(n) = 0; n) {}
// init-statement expressions
- if (T{f()}; f()) {}
- if (T{f()}, g, h; f()) {} // expected-warning 2{{unused}}
- if (T(f()), g, h + 1; f()) {} // expected-warning 2{{unused}}
+ if (T{f()}; f()) {} // expected-warning {{expression result unused}}
+ if (T{f()}, g, h; f()) {} // expected-warning 2{{unused}} expected-warning {{expression result unused}}
+ if (T(f()), g, h + 1; f()) {} // expected-warning 2{{unused}} expected-warning {{expression result unused}}
// condition declarations
if (T(n){g}) {}
@@ -35,7 +35,7 @@
// Likewise for 'switch'
switch (int n; n) {}
- switch (g; int g = 5) {}
+ switch (g; int g = 5) {} // expected-warning {{expression result unused}}
if (int a, b; int c = a) { // expected-note 6{{previous}}
int a; // expected-error {{redefinition}}
diff --git a/clang/test/Parser/switch-recovery.cpp b/clang/test/Parser/switch-recovery.cpp
index a3a0178..eacd017 100644
--- a/clang/test/Parser/switch-recovery.cpp
+++ b/clang/test/Parser/switch-recovery.cpp
@@ -105,7 +105,7 @@
expected-error {{expected expression}}
8:: x; // expected-error {{expected ';' after expression}} \
expected-error {{no member named 'x' in the global namespace; did you mean simply 'x'?}} \
- expected-warning 2 {{expression result unused}}
+ expected-warning {{expression result unused}}
9:: :y; // expected-error {{expected ';' after expression}} \
expected-error {{expected unqualified-id}} \
expected-warning {{expression result unused}}
diff --git a/clang/test/SemaCXX/cxx1z-init-statement.cpp b/clang/test/SemaCXX/cxx1z-init-statement.cpp
index d37acd0..eea2589 100644
--- a/clang/test/SemaCXX/cxx1z-init-statement.cpp
+++ b/clang/test/SemaCXX/cxx1z-init-statement.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++1z -verify %s
-// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -Wno-unused-value -verify %s
+// RUN: %clang_cc1 -std=c++17 -Wno-unused-value -verify %s
void testIf() {
int x = 0;
@@ -12,7 +12,7 @@
int x = 0; // expected-error {{redefinition of 'x'}}
if (x; int a = 0) ++a;
- if (x, +x; int a = 0) // expected-note 2 {{previous definition is here}} expected-warning {{unused}}
+ if (x, +x; int a = 0) // expected-note 2 {{previous definition is here}}
int a = 0; // expected-error {{redefinition of 'a'}}
else
int a = 0; // expected-error {{redefinition of 'a'}}
@@ -48,7 +48,7 @@
++a;
}
- switch (x, +x; int a = 0) { // expected-note {{previous definition is here}} expected-warning {{unused}}
+ switch (x, +x; int a = 0) { // expected-note {{previous definition is here}}
case 0:
int a = 0; // expected-error {{redefinition of 'a'}} // expected-note {{previous definition is here}}
case 1:
diff --git a/clang/test/SemaCXX/for-range-examples.cpp b/clang/test/SemaCXX/for-range-examples.cpp
index 477789b..5424b7a 100644
--- a/clang/test/SemaCXX/for-range-examples.cpp
+++ b/clang/test/SemaCXX/for-range-examples.cpp
@@ -244,7 +244,7 @@
{
int b = 1, a[b];
a[0] = 0;
- [&] { for (int c : a) 0; } ();
+ [&] { for (int c : a) 0; } (); // expected-warning {{expression result unused}}
}
diff --git a/clang/test/SemaCXX/warn-unused-result.cpp b/clang/test/SemaCXX/warn-unused-result.cpp
index 88f5ab1..f1de461 100644
--- a/clang/test/SemaCXX/warn-unused-result.cpp
+++ b/clang/test/SemaCXX/warn-unused-result.cpp
@@ -33,6 +33,36 @@
const S &s4 = g1();
}
+void testSubstmts(int i) {
+ switch (i) {
+ case 0:
+ f(); // expected-warning {{ignoring return value}}
+ default:
+ f(); // expected-warning {{ignoring return value}}
+ }
+
+ if (i)
+ f(); // expected-warning {{ignoring return value}}
+ else
+ f(); // expected-warning {{ignoring return value}}
+
+ while (i)
+ f(); // expected-warning {{ignoring return value}}
+
+ do
+ f(); // expected-warning {{ignoring return value}}
+ while (i);
+
+ for (f(); // expected-warning {{ignoring return value}}
+ ;
+ f() // expected-warning {{ignoring return value}}
+ )
+ f(); // expected-warning {{ignoring return value}}
+
+ f(), // expected-warning {{ignoring return value}}
+ (void)f();
+}
+
struct X {
int foo() __attribute__((warn_unused_result));
};
@@ -206,3 +236,13 @@
(void)++p;
}
} // namespace
+
+namespace PR39837 {
+[[clang::warn_unused_result]] int f(int);
+
+void g() {
+ int a[2];
+ for (int b : a)
+ f(b); // expected-warning {{ignoring return value}}
+}
+} // namespace PR39837
