[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

shafik added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:322 + auto EndLoc = Args.empty() ? Loc : Args.back()->getEndLoc(); + return S.BuildCallExpr(nullptr, Result.get(), Loc, Args, EndLoc, nullptr); } We should use [bugprone-argument-comme

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

This revision was automatically updated to reflect the committed changes. Closed by commit rG8d60e10ce4bd: [AST][Coroutine] Fix CoyieldExpr missing end loc (authored by dingfei ). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157296/

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 added a comment. I'll finish this patch when CI build succeeds. For future improvement I might start with the idea of marking those generated inner exprs as implicit, this seems to be easier. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157296/new/ https://reviews.llvm.org/D1

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 updated this revision to Diff 548434. danix800 edited the summary of this revision. danix800 added a comment. Cleanup duplicated boilerplate testcase code. Append a few extra child nodes of `CoyieldExpr` dumped in testcase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157296/n

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

aaron.ballman added a comment. Aside from some changes to the test coverage, I think @hokein and I are in agreement on moving forward with the patch as-is (feel free to correct me if I'm wrong though!). Comment at: clang/lib/Sema/SemaCoroutine.cpp:322 + auto EndLoc = Args.em

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

hokein added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:322 + auto EndLoc = Args.empty() ? Loc : Args.back()->getEndLoc(); + return S.BuildCallExpr(nullptr, Result.get(), Loc, Args, EndLoc, nullptr); } aaron.ballman wrote: > hokein wrote: >

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:322 + auto EndLoc = Args.empty() ? Loc : Args.back()->getEndLoc(); + return S.BuildCallExpr(nullptr, Result.get(), Loc, Args, EndLoc, nullptr); } hokein wrote: > aaron.ballman wr

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

hokein added a comment. Thanks for exploring all the options. For case 1) > Note that CXXMemberCallExpr 0x55d106716260 > relies on > RParenLoc directly. Coroutine is implemented as member call so I think RParen > is intended and should exist and be correct. Yeah, I think this is because we do

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 added a comment. 1. Use invalid loc: `SourceLocation()` ExprWithCleanups 0x55d1067162c8 'void' `-CoyieldExpr 0x55d106716280 'void' |-CXXMemberCallExpr 0x55d106715ed8 'std::suspend_always':'std::suspend_always' | |-MemberExpr 0x55d106715ea8 '' .yield_value 0x55d10670b220

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:322 + auto EndLoc = Args.empty() ? Loc : Args.back()->getEndLoc(); + return S.BuildCallExpr(nullptr, Result.get(), Loc, Args, EndLoc, nullptr); } hokein wrote: > Thanks for the f

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

hokein added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:322 + auto EndLoc = Args.empty() ? Loc : Args.back()->getEndLoc(); + return S.BuildCallExpr(nullptr, Result.get(), Loc, Args, EndLoc, nullptr); } Thanks for the fast fix. Reading the s

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157296/new/ https://reviews.llvm.org/D157296

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 updated this revision to Diff 547843. danix800 added a comment. Use `getEndLoc()` instead of `getBeginLoc()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157296/new/ https://reviews.llvm.org/D157296 Files: clang/lib/Sema/SemaCoroutine

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:321 - return S.BuildCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr); + auto EndLoc = Args.empty() ? Loc : Args.back()->getBeginLoc(); + return S.BuildCallExpr(nullptr, Result.get(), Loc, Ar

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

tbaeder added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:321 - return S.BuildCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr); + auto EndLoc = Args.empty() ? Loc : Args.back()->getBeginLoc(); + return S.BuildCallExpr(nullptr, Result.get(), Loc, Arg

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

danix800 created this revision. danix800 added a reviewer: aaron.ballman. danix800 added a project: clang. Herald added a subscriber: ChuanqiXu. Herald added a project: All. danix800 requested review of this revision. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-proje