[coroutines] Fix fallthrough diagnostics for coroutines
Summary: This patch fixes a number of issues with the analysis warnings emitted when a coroutine may reach the end of the function w/o returning. * Fix bug where coroutines with `return_value` are incorrectly diagnosed as missing `co_return`'s. * Rework diagnostic message to no longer say "non-void coroutine", because that implies the coroutine doesn't have a void return type, which it might. In this case a non-void coroutine is one who's promise type does not contain `return_void()` As a side-effect of this patch, coroutine bodies that contain an invalid coroutine promise objects are marked as invalid. Reviewers: GorNishanov, rsmith, aaron.ballman, majnemer Reviewed By: GorNishanov Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D33532 llvm-svn: 303831
This commit is contained in:
parent
1754fee864
commit
da8f9b5b1b
|
@ -537,10 +537,10 @@ def err_maybe_falloff_nonvoid_block : Error<
|
||||||
def err_falloff_nonvoid_block : Error<
|
def err_falloff_nonvoid_block : Error<
|
||||||
"control reaches end of non-void block">;
|
"control reaches end of non-void block">;
|
||||||
def warn_maybe_falloff_nonvoid_coroutine : Warning<
|
def warn_maybe_falloff_nonvoid_coroutine : Warning<
|
||||||
"control may reach end of non-void coroutine">,
|
"control may reach end of coroutine; which is undefined behavior because the promise type %0 does not declare 'return_void()'">,
|
||||||
InGroup<ReturnType>;
|
InGroup<ReturnType>;
|
||||||
def warn_falloff_nonvoid_coroutine : Warning<
|
def warn_falloff_nonvoid_coroutine : Warning<
|
||||||
"control reaches end of non-void coroutine">,
|
"control reaches end of coroutine; which is undefined behavior because the promise type %0 does not declare 'return_void()'">,
|
||||||
InGroup<ReturnType>;
|
InGroup<ReturnType>;
|
||||||
def warn_suggest_noreturn_function : Warning<
|
def warn_suggest_noreturn_function : Warning<
|
||||||
"%select{function|method}0 %1 could be declared with attribute 'noreturn'">,
|
"%select{function|method}0 %1 could be declared with attribute 'noreturn'">,
|
||||||
|
|
|
@ -388,6 +388,8 @@ public:
|
||||||
(HasBranchProtectedScope && HasBranchIntoScope));
|
(HasBranchProtectedScope && HasBranchIntoScope));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool isCoroutine() const { return !FirstCoroutineStmtLoc.isInvalid(); }
|
||||||
|
|
||||||
void setFirstCoroutineStmt(SourceLocation Loc, StringRef Keyword) {
|
void setFirstCoroutineStmt(SourceLocation Loc, StringRef Keyword) {
|
||||||
assert(FirstCoroutineStmtLoc.isInvalid() &&
|
assert(FirstCoroutineStmtLoc.isInvalid() &&
|
||||||
"first coroutine statement location already set");
|
"first coroutine statement location already set");
|
||||||
|
|
|
@ -92,6 +92,8 @@ Stmt *AnalysisDeclContext::getBody(bool &IsAutosynthesized) const {
|
||||||
IsAutosynthesized = false;
|
IsAutosynthesized = false;
|
||||||
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
|
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
|
||||||
Stmt *Body = FD->getBody();
|
Stmt *Body = FD->getBody();
|
||||||
|
if (auto *CoroBody = dyn_cast_or_null<CoroutineBodyStmt>(Body))
|
||||||
|
Body = CoroBody->getBody();
|
||||||
if (Manager && Manager->synthesizeBodies()) {
|
if (Manager && Manager->synthesizeBodies()) {
|
||||||
Stmt *SynthesizedBody =
|
Stmt *SynthesizedBody =
|
||||||
getBodyFarm(getASTContext(), Manager->Injector.get()).getBody(FD);
|
getBodyFarm(getASTContext(), Manager->Injector.get()).getBody(FD);
|
||||||
|
|
|
@ -334,10 +334,6 @@ static ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC) {
|
||||||
bool HasPlainEdge = false;
|
bool HasPlainEdge = false;
|
||||||
bool HasAbnormalEdge = false;
|
bool HasAbnormalEdge = false;
|
||||||
|
|
||||||
// In a coroutine, only co_return statements count as normal returns. Remember
|
|
||||||
// if we are processing a coroutine or not.
|
|
||||||
const bool IsCoroutine = isa<CoroutineBodyStmt>(AC.getBody());
|
|
||||||
|
|
||||||
// Ignore default cases that aren't likely to be reachable because all
|
// Ignore default cases that aren't likely to be reachable because all
|
||||||
// enums in a switch(X) have explicit case statements.
|
// enums in a switch(X) have explicit case statements.
|
||||||
CFGBlock::FilterOptions FO;
|
CFGBlock::FilterOptions FO;
|
||||||
|
@ -379,7 +375,7 @@ static ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC) {
|
||||||
|
|
||||||
CFGStmt CS = ri->castAs<CFGStmt>();
|
CFGStmt CS = ri->castAs<CFGStmt>();
|
||||||
const Stmt *S = CS.getStmt();
|
const Stmt *S = CS.getStmt();
|
||||||
if ((isa<ReturnStmt>(S) && !IsCoroutine) || isa<CoreturnStmt>(S)) {
|
if (isa<ReturnStmt>(S) || isa<CoreturnStmt>(S)) {
|
||||||
HasLiveReturn = true;
|
HasLiveReturn = true;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
@ -546,6 +542,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
|
||||||
|
|
||||||
bool ReturnsVoid = false;
|
bool ReturnsVoid = false;
|
||||||
bool HasNoReturn = false;
|
bool HasNoReturn = false;
|
||||||
|
bool IsCoroutine = S.getCurFunction() && S.getCurFunction()->isCoroutine();
|
||||||
|
|
||||||
if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
|
if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
|
||||||
if (const auto *CBody = dyn_cast<CoroutineBodyStmt>(Body))
|
if (const auto *CBody = dyn_cast<CoroutineBodyStmt>(Body))
|
||||||
|
@ -574,8 +571,13 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
|
||||||
// Short circuit for compilation speed.
|
// Short circuit for compilation speed.
|
||||||
if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn))
|
if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
SourceLocation LBrace = Body->getLocStart(), RBrace = Body->getLocEnd();
|
SourceLocation LBrace = Body->getLocStart(), RBrace = Body->getLocEnd();
|
||||||
|
auto EmitDiag = [&](SourceLocation Loc, unsigned DiagID) {
|
||||||
|
if (IsCoroutine)
|
||||||
|
S.Diag(Loc, DiagID) << S.getCurFunction()->CoroutinePromise->getType();
|
||||||
|
else
|
||||||
|
S.Diag(Loc, DiagID);
|
||||||
|
};
|
||||||
// Either in a function body compound statement, or a function-try-block.
|
// Either in a function body compound statement, or a function-try-block.
|
||||||
switch (CheckFallThrough(AC)) {
|
switch (CheckFallThrough(AC)) {
|
||||||
case UnknownFallThrough:
|
case UnknownFallThrough:
|
||||||
|
@ -583,15 +585,15 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
|
||||||
|
|
||||||
case MaybeFallThrough:
|
case MaybeFallThrough:
|
||||||
if (HasNoReturn)
|
if (HasNoReturn)
|
||||||
S.Diag(RBrace, CD.diag_MaybeFallThrough_HasNoReturn);
|
EmitDiag(RBrace, CD.diag_MaybeFallThrough_HasNoReturn);
|
||||||
else if (!ReturnsVoid)
|
else if (!ReturnsVoid)
|
||||||
S.Diag(RBrace, CD.diag_MaybeFallThrough_ReturnsNonVoid);
|
EmitDiag(RBrace, CD.diag_MaybeFallThrough_ReturnsNonVoid);
|
||||||
break;
|
break;
|
||||||
case AlwaysFallThrough:
|
case AlwaysFallThrough:
|
||||||
if (HasNoReturn)
|
if (HasNoReturn)
|
||||||
S.Diag(RBrace, CD.diag_AlwaysFallThrough_HasNoReturn);
|
EmitDiag(RBrace, CD.diag_AlwaysFallThrough_HasNoReturn);
|
||||||
else if (!ReturnsVoid)
|
else if (!ReturnsVoid)
|
||||||
S.Diag(RBrace, CD.diag_AlwaysFallThrough_ReturnsNonVoid);
|
EmitDiag(RBrace, CD.diag_AlwaysFallThrough_ReturnsNonVoid);
|
||||||
break;
|
break;
|
||||||
case NeverFallThroughOrReturn:
|
case NeverFallThroughOrReturn:
|
||||||
if (ReturnsVoid && !HasNoReturn && CD.diag_NeverFallThroughOrReturn) {
|
if (ReturnsVoid && !HasNoReturn && CD.diag_NeverFallThroughOrReturn) {
|
||||||
|
@ -2031,12 +2033,6 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
|
||||||
|
|
||||||
// Warning: check missing 'return'
|
// Warning: check missing 'return'
|
||||||
if (P.enableCheckFallThrough) {
|
if (P.enableCheckFallThrough) {
|
||||||
auto IsCoro = [&]() {
|
|
||||||
if (auto *FD = dyn_cast<FunctionDecl>(D))
|
|
||||||
if (FD->getBody() && isa<CoroutineBodyStmt>(FD->getBody()))
|
|
||||||
return true;
|
|
||||||
return false;
|
|
||||||
};
|
|
||||||
const CheckFallThroughDiagnostics &CD =
|
const CheckFallThroughDiagnostics &CD =
|
||||||
(isa<BlockDecl>(D)
|
(isa<BlockDecl>(D)
|
||||||
? CheckFallThroughDiagnostics::MakeForBlock()
|
? CheckFallThroughDiagnostics::MakeForBlock()
|
||||||
|
@ -2044,7 +2040,7 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
|
||||||
cast<CXXMethodDecl>(D)->getOverloadedOperator() == OO_Call &&
|
cast<CXXMethodDecl>(D)->getOverloadedOperator() == OO_Call &&
|
||||||
cast<CXXMethodDecl>(D)->getParent()->isLambda())
|
cast<CXXMethodDecl>(D)->getParent()->isLambda())
|
||||||
? CheckFallThroughDiagnostics::MakeForLambda()
|
? CheckFallThroughDiagnostics::MakeForLambda()
|
||||||
: (IsCoro()
|
: (fscope->isCoroutine()
|
||||||
? CheckFallThroughDiagnostics::MakeForCoroutine(D)
|
? CheckFallThroughDiagnostics::MakeForCoroutine(D)
|
||||||
: CheckFallThroughDiagnostics::MakeForFunction(D)));
|
: CheckFallThroughDiagnostics::MakeForFunction(D)));
|
||||||
CheckFallThroughForBody(S, D, Body, blkExpr, CD, AC);
|
CheckFallThroughForBody(S, D, Body, blkExpr, CD, AC);
|
||||||
|
|
|
@ -719,13 +719,16 @@ static FunctionDecl *findDeleteForPromise(Sema &S, SourceLocation Loc,
|
||||||
|
|
||||||
void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) {
|
void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) {
|
||||||
FunctionScopeInfo *Fn = getCurFunction();
|
FunctionScopeInfo *Fn = getCurFunction();
|
||||||
assert(Fn && Fn->CoroutinePromise && "not a coroutine");
|
assert(Fn && Fn->isCoroutine() && "not a coroutine");
|
||||||
|
|
||||||
if (!Body) {
|
if (!Body) {
|
||||||
assert(FD->isInvalidDecl() &&
|
assert(FD->isInvalidDecl() &&
|
||||||
"a null body is only allowed for invalid declarations");
|
"a null body is only allowed for invalid declarations");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
// We have a function that uses coroutine keywords, but we failed to build
|
||||||
|
// the promise type.
|
||||||
|
if (!Fn->CoroutinePromise)
|
||||||
|
return FD->setInvalidDecl();
|
||||||
|
|
||||||
if (isa<CoroutineBodyStmt>(Body)) {
|
if (isa<CoroutineBodyStmt>(Body)) {
|
||||||
// Nothing todo. the body is already a transformed coroutine body statement.
|
// Nothing todo. the body is already a transformed coroutine body statement.
|
||||||
|
|
|
@ -12179,7 +12179,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
|
||||||
sema::AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy();
|
sema::AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy();
|
||||||
sema::AnalysisBasedWarnings::Policy *ActivePolicy = nullptr;
|
sema::AnalysisBasedWarnings::Policy *ActivePolicy = nullptr;
|
||||||
|
|
||||||
if (getLangOpts().CoroutinesTS && getCurFunction()->CoroutinePromise)
|
if (getLangOpts().CoroutinesTS && getCurFunction()->isCoroutine())
|
||||||
CheckCompletedCoroutineBody(FD, Body);
|
CheckCompletedCoroutineBody(FD, Body);
|
||||||
|
|
||||||
if (FD) {
|
if (FD) {
|
||||||
|
|
|
@ -18,6 +18,43 @@ struct promise_void {
|
||||||
void unhandled_exception();
|
void unhandled_exception();
|
||||||
};
|
};
|
||||||
|
|
||||||
|
struct promise_void_return_value {
|
||||||
|
void get_return_object();
|
||||||
|
suspend_always initial_suspend();
|
||||||
|
suspend_always final_suspend();
|
||||||
|
void unhandled_exception();
|
||||||
|
void return_value(int);
|
||||||
|
};
|
||||||
|
|
||||||
|
struct VoidTagNoReturn {
|
||||||
|
struct promise_type {
|
||||||
|
VoidTagNoReturn get_return_object();
|
||||||
|
suspend_always initial_suspend();
|
||||||
|
suspend_always final_suspend();
|
||||||
|
void unhandled_exception();
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
|
struct VoidTagReturnValue {
|
||||||
|
struct promise_type {
|
||||||
|
VoidTagReturnValue get_return_object();
|
||||||
|
suspend_always initial_suspend();
|
||||||
|
suspend_always final_suspend();
|
||||||
|
void unhandled_exception();
|
||||||
|
void return_value(int);
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
|
struct VoidTagReturnVoid {
|
||||||
|
struct promise_type {
|
||||||
|
VoidTagReturnVoid get_return_object();
|
||||||
|
suspend_always initial_suspend();
|
||||||
|
suspend_always final_suspend();
|
||||||
|
void unhandled_exception();
|
||||||
|
void return_void();
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
struct promise_float {
|
struct promise_float {
|
||||||
float get_return_object();
|
float get_return_object();
|
||||||
suspend_always initial_suspend();
|
suspend_always initial_suspend();
|
||||||
|
@ -34,8 +71,11 @@ struct promise_int {
|
||||||
void unhandled_exception();
|
void unhandled_exception();
|
||||||
};
|
};
|
||||||
|
|
||||||
template <typename... T>
|
template <>
|
||||||
struct std::experimental::coroutine_traits<void, T...> { using promise_type = promise_void; };
|
struct std::experimental::coroutine_traits<void> { using promise_type = promise_void; };
|
||||||
|
|
||||||
|
template <typename T1>
|
||||||
|
struct std::experimental::coroutine_traits<void, T1> { using promise_type = promise_void_return_value; };
|
||||||
|
|
||||||
template <typename... T>
|
template <typename... T>
|
||||||
struct std::experimental::coroutine_traits<float, T...> { using promise_type = promise_float; };
|
struct std::experimental::coroutine_traits<float, T...> { using promise_type = promise_float; };
|
||||||
|
@ -48,10 +88,66 @@ float test1() { co_await a; }
|
||||||
|
|
||||||
int test2() {
|
int test2() {
|
||||||
co_await a;
|
co_await a;
|
||||||
} // expected-warning {{control reaches end of non-void coroutine}}
|
} // expected-warning {{control reaches end of coroutine; which is undefined behavior because the promise type 'std::experimental::coroutine_traits<int>::promise_type' (aka 'promise_int') does not declare 'return_void()'}}
|
||||||
|
|
||||||
|
int test2a(bool b) {
|
||||||
|
if (b)
|
||||||
|
co_return 42;
|
||||||
|
} // expected-warning {{control may reach end of coroutine; which is undefined behavior because the promise type 'std::experimental::coroutine_traits<int, bool>::promise_type' (aka 'promise_int') does not declare 'return_void()'}}
|
||||||
|
|
||||||
int test3() {
|
int test3() {
|
||||||
co_await a;
|
co_await a;
|
||||||
b:
|
b:
|
||||||
goto b;
|
goto b;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
int test4() {
|
||||||
|
co_return 42;
|
||||||
|
}
|
||||||
|
|
||||||
|
void test5(int) {
|
||||||
|
co_await a;
|
||||||
|
} // expected-warning {{control reaches end of coroutine; which is undefined behavior because}}
|
||||||
|
|
||||||
|
void test6(int x) {
|
||||||
|
if (x)
|
||||||
|
co_return 42;
|
||||||
|
} // expected-warning {{control may reach end of coroutine; which is undefined behavior because}}
|
||||||
|
|
||||||
|
void test7(int y) {
|
||||||
|
if (y)
|
||||||
|
co_return 42;
|
||||||
|
else
|
||||||
|
co_return 101;
|
||||||
|
}
|
||||||
|
|
||||||
|
VoidTagReturnVoid test8() {
|
||||||
|
co_await a;
|
||||||
|
}
|
||||||
|
|
||||||
|
VoidTagReturnVoid test9(bool b) {
|
||||||
|
if (b)
|
||||||
|
co_return;
|
||||||
|
}
|
||||||
|
|
||||||
|
VoidTagReturnValue test10() {
|
||||||
|
co_await a;
|
||||||
|
} // expected-warning {{control reaches end of coroutine}}
|
||||||
|
|
||||||
|
VoidTagReturnValue test11(bool b) {
|
||||||
|
if (b)
|
||||||
|
co_return 42;
|
||||||
|
} // expected-warning {{control may reach end of coroutine}}
|
||||||
|
|
||||||
|
// FIXME: Promise types that declare neither return_value nor return_void
|
||||||
|
// should be ill-formed. This test should be removed when that is fixed.
|
||||||
|
VoidTagNoReturn test12() {
|
||||||
|
co_await a;
|
||||||
|
} // expected-warning {{control reaches end of coroutine}}
|
||||||
|
|
||||||
|
// FIXME: Promise types that declare neither return_value nor return_void
|
||||||
|
// should be ill-formed. This test should be removed when that is fixed.
|
||||||
|
VoidTagNoReturn test13(bool b) {
|
||||||
|
if (b)
|
||||||
|
co_await a;
|
||||||
|
} // expected-warning {{control reaches end of coroutine}}
|
||||||
|
|
|
@ -818,3 +818,14 @@ extern "C" int f(mismatch_gro_type_tag4) {
|
||||||
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
|
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
struct bad_promise_no_return_func {
|
||||||
|
coro<bad_promise_no_return_func> get_return_object();
|
||||||
|
suspend_always initial_suspend();
|
||||||
|
suspend_always final_suspend();
|
||||||
|
void unhandled_exception();
|
||||||
|
};
|
||||||
|
// FIXME: Make this ill-formed because the promise type declares
|
||||||
|
// neither return_value(...) or return_void().
|
||||||
|
coro<bad_promise_no_return_func> no_return_value_or_return_void() {
|
||||||
|
co_await a;
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in New Issue