EricWF added inline comments.
================
Comment at: include/clang/Sema/ScopeInfo.h:138-140
+ /// \brief Whether this function has already built, or tried to build, the
+ /// the initial and final coroutine suspend points.
+ bool NeedsCoroutineSuspends : 1;
----------------
rsmith wrote:
> Is the comment here correct? It seems a slightly odd match for the member
> name.
The comment should probably say "True only when this function has not already
built, or attempted to build, the initial and final coroutine suspend points".
================
Comment at: lib/Sema/SemaCoroutine.cpp:99
+
+ auto buildNNS = [&]() {
auto *NNS = NestedNameSpecifier::Create(S.Context, nullptr, StdExp);
----------------
rsmith wrote:
> `buildElaboratedType` would be a better name for what this does. I also
> wonder if this is really necessary, or whether we can just use %q0 in the
> diagnostic format to request a fully-qualified type name.
We can't use "%q0" in the diagnostic passed to `RequireCompleteType`, so we
have to build up the elaborated `QualType` instead.
================
Comment at: lib/Sema/SemaCoroutine.cpp:112-116
+ if (S.RequireCompleteType(FuncLoc, buildNNS(),
+ diag::err_coroutine_promise_type_incomplete))
+ return QualType();
return PromiseType;
----------------
rsmith wrote:
> We used to return the `ElaboratedType` and don't do so any more.
I don't think that's true. We only build the `ElaboratedType` before issuing a
diagnostic and returning `QualType()` to signal failure.
================
Comment at: lib/Sema/SemaCoroutine.cpp:33
/// function type.
static QualType lookupPromiseType(Sema &S, const FunctionProtoType *FnType,
+ SourceLocation KwLoc,
----------------
rsmith wrote:
> rsmith wrote:
> > EricWF wrote:
> > > The changes to this function are all unrelated cleanup/improvements.
> > Just go ahead and commit these separately then.
> Please commit these changes separately if they're cleanups unrelated to the
> main purpose of the patch.
Ack.
================
Comment at: lib/Sema/TreeTransform.h:6802
+ return getDerived().RebuildDependentCoawaitExpr(
+ E->getKeywordLoc(), Result.get(), E->getOperatorCoawaitLookup());
+}
----------------
rsmith wrote:
> You need to transform the UnresolvedLookupExpr here too. (Example: we might
> have a function-local declaration of `operator co_await` that needs to be
> transformed into the version in the instantiated function.)
Ack. Fixed.
Do you have an example that doesn't use a function-local definition? Since
`operator co_await` cannot be defined locally.
https://reviews.llvm.org/D26057
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
