Skip to content

Commit

Permalink
[coroutines] Fix co_await for range statement
Browse files Browse the repository at this point in the history
Summary:
Currently we build the co_await expressions on the wrong implicit statements of the implicit ranged for; Specifically we build the co_await expression wrapping the range declaration, but it should wrap the begin expression.

This patch fixes co_await on range for.

Reviewers: rsmith, GorNishanov

Reviewed By: GorNishanov

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D34021

llvm-svn: 305363
  • Loading branch information
EricWF committed Jun 14, 2017
1 parent ee64f5e commit b936a39
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 37 deletions.
2 changes: 2 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -8364,6 +8364,8 @@ class Sema {
//===--------------------------------------------------------------------===//
// C++ Coroutines TS
//
bool ActOnCoroutineBodyStart(Scope *S, SourceLocation KwLoc,
StringRef Keyword);
ExprResult ActOnCoawaitExpr(Scope *S, SourceLocation KwLoc, Expr *E);
ExprResult ActOnCoyieldExpr(Scope *S, SourceLocation KwLoc, Expr *E);
StmtResult ActOnCoreturnStmt(Scope *S, SourceLocation KwLoc, Expr *E);
Expand Down
35 changes: 18 additions & 17 deletions clang/lib/Sema/SemaCoroutine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,11 @@ static FunctionScopeInfo *checkCoroutineContext(Sema &S, SourceLocation Loc,
return ScopeInfo;
}

static bool actOnCoroutineBodyStart(Sema &S, Scope *SC, SourceLocation KWLoc,
StringRef Keyword) {
if (!checkCoroutineContext(S, KWLoc, Keyword))
bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc,
StringRef Keyword) {
if (!checkCoroutineContext(*this, KWLoc, Keyword))
return false;
auto *ScopeInfo = S.getCurFunction();
auto *ScopeInfo = getCurFunction();
assert(ScopeInfo->CoroutinePromise);

// If we have existing coroutine statements then we have already built
Expand All @@ -484,24 +484,24 @@ static bool actOnCoroutineBodyStart(Sema &S, Scope *SC, SourceLocation KWLoc,

ScopeInfo->setNeedsCoroutineSuspends(false);

auto *Fn = cast<FunctionDecl>(S.CurContext);
auto *Fn = cast<FunctionDecl>(CurContext);
SourceLocation Loc = Fn->getLocation();
// Build the initial suspend point
auto buildSuspends = [&](StringRef Name) mutable -> StmtResult {
ExprResult Suspend =
buildPromiseCall(S, ScopeInfo->CoroutinePromise, Loc, Name, None);
buildPromiseCall(*this, ScopeInfo->CoroutinePromise, Loc, Name, None);
if (Suspend.isInvalid())
return StmtError();
Suspend = buildOperatorCoawaitCall(S, SC, Loc, Suspend.get());
Suspend = buildOperatorCoawaitCall(*this, SC, Loc, Suspend.get());
if (Suspend.isInvalid())
return StmtError();
Suspend = S.BuildResolvedCoawaitExpr(Loc, Suspend.get(),
/*IsImplicit*/ true);
Suspend = S.ActOnFinishFullExpr(Suspend.get());
Suspend = BuildResolvedCoawaitExpr(Loc, Suspend.get(),
/*IsImplicit*/ true);
Suspend = ActOnFinishFullExpr(Suspend.get());
if (Suspend.isInvalid()) {
S.Diag(Loc, diag::note_coroutine_promise_suspend_implicitly_required)
Diag(Loc, diag::note_coroutine_promise_suspend_implicitly_required)
<< ((Name == "initial_suspend") ? 0 : 1);
S.Diag(KWLoc, diag::note_declared_coroutine_here) << Keyword;
Diag(KWLoc, diag::note_declared_coroutine_here) << Keyword;
return StmtError();
}
return cast<Stmt>(Suspend.get());
Expand All @@ -521,7 +521,7 @@ static bool actOnCoroutineBodyStart(Sema &S, Scope *SC, SourceLocation KWLoc,
}

ExprResult Sema::ActOnCoawaitExpr(Scope *S, SourceLocation Loc, Expr *E) {
if (!actOnCoroutineBodyStart(*this, S, Loc, "co_await")) {
if (!ActOnCoroutineBodyStart(S, Loc, "co_await")) {
CorrectDelayedTyposInExpr(E);
return ExprError();
}
Expand Down Expand Up @@ -613,7 +613,7 @@ ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation Loc, Expr *E,
}

ExprResult Sema::ActOnCoyieldExpr(Scope *S, SourceLocation Loc, Expr *E) {
if (!actOnCoroutineBodyStart(*this, S, Loc, "co_yield")) {
if (!ActOnCoroutineBodyStart(S, Loc, "co_yield")) {
CorrectDelayedTyposInExpr(E);
return ExprError();
}
Expand Down Expand Up @@ -658,14 +658,15 @@ ExprResult Sema::BuildCoyieldExpr(SourceLocation Loc, Expr *E) {
if (RSS.IsInvalid)
return ExprError();

Expr *Res = new (Context) CoyieldExpr(Loc, E, RSS.Results[0], RSS.Results[1],
RSS.Results[2], RSS.OpaqueValue);
Expr *Res =
new (Context) CoyieldExpr(Loc, E, RSS.Results[0], RSS.Results[1],
RSS.Results[2], RSS.OpaqueValue);

return Res;
}

StmtResult Sema::ActOnCoreturnStmt(Scope *S, SourceLocation Loc, Expr *E) {
if (!actOnCoroutineBodyStart(*this, S, Loc, "co_return")) {
if (!ActOnCoroutineBodyStart(S, Loc, "co_return")) {
CorrectDelayedTyposInExpr(E);
return StmtError();
}
Expand Down
52 changes: 32 additions & 20 deletions clang/lib/Sema/SemaStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1989,11 +1989,11 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
return StmtError();
}

// Coroutines: 'for co_await' implicitly co_awaits its range.
if (CoawaitLoc.isValid()) {
ExprResult Coawait = ActOnCoawaitExpr(S, CoawaitLoc, Range);
if (Coawait.isInvalid()) return StmtError();
Range = Coawait.get();
// Build the coroutine state immediately and not later during template
// instantiation
if (!CoawaitLoc.isInvalid()) {
if (!ActOnCoroutineBodyStart(S, CoawaitLoc, "co_await"))
return StmtError();
}

// Build auto && __range = range-init
Expand Down Expand Up @@ -2031,16 +2031,12 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
/// BeginExpr and EndExpr are set and FRS_Success is returned on success;
/// CandidateSet and BEF are set and some non-success value is returned on
/// failure.
static Sema::ForRangeStatus BuildNonArrayForRange(Sema &SemaRef,
Expr *BeginRange, Expr *EndRange,
QualType RangeType,
VarDecl *BeginVar,
VarDecl *EndVar,
SourceLocation ColonLoc,
OverloadCandidateSet *CandidateSet,
ExprResult *BeginExpr,
ExprResult *EndExpr,
BeginEndFunction *BEF) {
static Sema::ForRangeStatus
BuildNonArrayForRange(Sema &SemaRef, Expr *BeginRange, Expr *EndRange,
QualType RangeType, VarDecl *BeginVar, VarDecl *EndVar,
SourceLocation ColonLoc, SourceLocation CoawaitLoc,
OverloadCandidateSet *CandidateSet, ExprResult *BeginExpr,
ExprResult *EndExpr, BeginEndFunction *BEF) {
DeclarationNameInfo BeginNameInfo(
&SemaRef.PP.getIdentifierTable().get("begin"), ColonLoc);
DeclarationNameInfo EndNameInfo(&SemaRef.PP.getIdentifierTable().get("end"),
Expand Down Expand Up @@ -2087,6 +2083,15 @@ static Sema::ForRangeStatus BuildNonArrayForRange(Sema &SemaRef,
<< ColonLoc << BEF_begin << BeginRange->getType();
return RangeStatus;
}
if (!CoawaitLoc.isInvalid()) {
// FIXME: getCurScope() should not be used during template instantiation.
// We should pick up the set of unqualified lookup results for operator
// co_await during the initial parse.
*BeginExpr = SemaRef.ActOnCoawaitExpr(SemaRef.getCurScope(), ColonLoc,
BeginExpr->get());
if (BeginExpr->isInvalid())
return Sema::FRS_DiagnosticIssued;
}
if (FinishForRangeVarDecl(SemaRef, BeginVar, BeginExpr->get(), ColonLoc,
diag::err_for_range_iter_deduction_failure)) {
NoteForRangeBeginEndFunction(SemaRef, BeginExpr->get(), *BEF);
Expand Down Expand Up @@ -2253,6 +2258,11 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation CoawaitLoc,

// begin-expr is __range.
BeginExpr = BeginRangeRef;
if (!CoawaitLoc.isInvalid()) {
BeginExpr = ActOnCoawaitExpr(S, ColonLoc, BeginExpr.get());
if (BeginExpr.isInvalid())
return StmtError();
}
if (FinishForRangeVarDecl(*this, BeginVar, BeginRangeRef.get(), ColonLoc,
diag::err_for_range_iter_deduction_failure)) {
NoteForRangeBeginEndFunction(*this, BeginExpr.get(), BEF_begin);
Expand Down Expand Up @@ -2335,11 +2345,10 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation CoawaitLoc,
OverloadCandidateSet CandidateSet(RangeLoc,
OverloadCandidateSet::CSK_Normal);
BeginEndFunction BEFFailure;
ForRangeStatus RangeStatus =
BuildNonArrayForRange(*this, BeginRangeRef.get(),
EndRangeRef.get(), RangeType,
BeginVar, EndVar, ColonLoc, &CandidateSet,
&BeginExpr, &EndExpr, &BEFFailure);
ForRangeStatus RangeStatus = BuildNonArrayForRange(
*this, BeginRangeRef.get(), EndRangeRef.get(), RangeType, BeginVar,
EndVar, ColonLoc, CoawaitLoc, &CandidateSet, &BeginExpr, &EndExpr,
&BEFFailure);

if (Kind == BFRK_Build && RangeStatus == FRS_NoViableFunction &&
BEFFailure == BEF_begin) {
Expand Down Expand Up @@ -2436,6 +2445,9 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation CoawaitLoc,

IncrExpr = ActOnUnaryOp(S, ColonLoc, tok::plusplus, BeginRef.get());
if (!IncrExpr.isInvalid() && CoawaitLoc.isValid())
// FIXME: getCurScope() should not be used during template instantiation.
// We should pick up the set of unqualified lookup results for operator
// co_await during the initial parse.
IncrExpr = ActOnCoawaitExpr(S, CoawaitLoc, IncrExpr.get());
if (!IncrExpr.isInvalid())
IncrExpr = ActOnFinishFullExpr(IncrExpr.get());
Expand Down
165 changes: 165 additions & 0 deletions clang/test/SemaCXX/co_await-range-for.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts \
// RUN: -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify \
// RUN: -fblocks
#include "Inputs/std-coroutine.h"

using namespace std::experimental;


template <class Begin>
struct Awaiter {
bool await_ready();
void await_suspend(coroutine_handle<>);
Begin await_resume();
};

template <class Iter> struct BeginTag { BeginTag() = delete; };
template <class Iter> struct IncTag { IncTag() = delete; };

template <class Iter, bool Delete = false>
struct CoawaitTag { CoawaitTag() = delete; };

template <class T>
struct Iter {
using value_type = T;
using reference = T &;
using pointer = T *;

IncTag<Iter> operator++();
reference operator*();
pointer operator->();
};
template <class T> bool operator==(Iter<T>, Iter<T>);
template <class T> bool operator!=(Iter<T>, Iter<T>);

template <class T>
struct Range {
BeginTag<Iter<T>> begin();
Iter<T> end();
};

struct MyForLoopArrayAwaiter {
struct promise_type {
MyForLoopArrayAwaiter get_return_object() { return {}; }
void return_void();
void unhandled_exception();
suspend_never initial_suspend();
suspend_never final_suspend();
template <class T>
Awaiter<T *> await_transform(T *) = delete; // expected-note {{explicitly deleted}}
};
};
MyForLoopArrayAwaiter g() {
int arr[10] = {0};
for co_await(auto i : arr) {}
// expected-error@-1 {{call to deleted member function 'await_transform'}}
// expected-note@-2 {{'await_transform' implicitly required by 'co_await' here}}
}

struct ForLoopAwaiterBadBeginTransform {
struct promise_type {
ForLoopAwaiterBadBeginTransform get_return_object();
void return_void();
void unhandled_exception();
suspend_never initial_suspend();
suspend_never final_suspend();

template <class T>
Awaiter<T> await_transform(BeginTag<T>) = delete; // expected-note 1+ {{explicitly deleted}}

template <class T>
CoawaitTag<T> await_transform(IncTag<T>); // expected-note 1+ {{candidate}}
};
};
ForLoopAwaiterBadBeginTransform bad_begin() {
Range<int> R;
for co_await(auto i : R) {}
// expected-error@-1 {{call to deleted member function 'await_transform'}}
// expected-note@-2 {{'await_transform' implicitly required by 'co_await' here}}
}
template <class Dummy>
ForLoopAwaiterBadBeginTransform bad_begin_template(Dummy) {
Range<Dummy> R;
for co_await(auto i : R) {}
// expected-error@-1 {{call to deleted member function 'await_transform'}}
// expected-note@-2 {{'await_transform' implicitly required by 'co_await' here}}
}
template ForLoopAwaiterBadBeginTransform bad_begin_template(int); // expected-note {{requested here}}

template <class Iter>
Awaiter<Iter> operator co_await(CoawaitTag<Iter, true>) = delete;
// expected-note@-1 1+ {{explicitly deleted}}

struct ForLoopAwaiterBadIncTransform {
struct promise_type {
ForLoopAwaiterBadIncTransform get_return_object();
void return_void();
void unhandled_exception();
suspend_never initial_suspend();
suspend_never final_suspend();

template <class T>
Awaiter<T> await_transform(BeginTag<T> e);

template <class T>
CoawaitTag<T, true> await_transform(IncTag<T>);
};
};
ForLoopAwaiterBadIncTransform bad_inc_transform() {
Range<float> R;
for co_await(auto i : R) {}
// expected-error@-1 {{overload resolution selected deleted operator 'co_await'}}
// expected-note@-2 {{in implicit call to 'operator++' for iterator of type 'Range<float>'}}
}

template <class Dummy>
ForLoopAwaiterBadIncTransform bad_inc_transform_template(Dummy) {
Range<Dummy> R;
for co_await(auto i : R) {}
// expected-error@-1 {{overload resolution selected deleted operator 'co_await'}}
// expected-note@-2 {{in implicit call to 'operator++' for iterator of type 'Range<long>'}}
}
template ForLoopAwaiterBadIncTransform bad_inc_transform_template(long); // expected-note {{requested here}}

// Ensure we mark and check the function as a coroutine even if it's
// never instantiated.
template <class T>
constexpr void never_instant(T) {
static_assert(sizeof(T) != sizeof(T), "function should not be instantiated");
for co_await(auto i : foo(T{})) {}
// expected-error@-1 {{'co_await' cannot be used in a constexpr function}}
}

namespace NS {
struct ForLoopAwaiterCoawaitLookup {
struct promise_type {
ForLoopAwaiterCoawaitLookup get_return_object();
void return_void();
void unhandled_exception();
suspend_never initial_suspend();
suspend_never final_suspend();
template <class T>
CoawaitTag<T, false> await_transform(BeginTag<T> e);
template <class T>
Awaiter<T> await_transform(IncTag<T>);
};
};
} // namespace NS
using NS::ForLoopAwaiterCoawaitLookup;

template <class T>
ForLoopAwaiterCoawaitLookup test_coawait_lookup(T) {
Range<T> R;
for co_await(auto i : R) {}
// expected-error@-1 {{no member named 'await_ready' in 'CoawaitTag<Iter<int>, false>'}}
}
template ForLoopAwaiterCoawaitLookup test_coawait_lookup(int); // expected-note {{requested here}}

// FIXME: This test should fail as well since the newly declared operator co_await
// should not be found by lookup.
namespace NS2 {
template <class Iter>
Awaiter<Iter> operator co_await(CoawaitTag<Iter, false>);
}
using NS2::operator co_await;
template ForLoopAwaiterCoawaitLookup test_coawait_lookup(long);

0 comments on commit b936a39

Please sign in to comment.