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

[Aaron Ballman via Phabricator via cfe-commits]

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 fast fix.
> 
> Reading the source code here, I'm not sure this is a correct fix (and put the heuristic here). I assume the `Loc` points the the `co_yield` token here, passing the `Loc` to the `LParenLoc` and `RParenLoc` parameters seems wrong to me, there is no `(` and `)` written in the source code here (because the member call expression is an implicit generated node in the clang AST), we should pass an invalid source location (`SourceLocation()`).
> 
> The `CallExpr::getEndLoc()` has implemented similar [heuristic](https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/Expr.cpp#L1662-L1664) to handle the case where the RParenLoc is invalid.
>  passing the Loc to the LParenLoc and RParenLoc parameters seems wrong to me

FWIW, I thought so as well, but I thought we have the same behavior for `CoreturnExpr` and `CoawaitExpr`. These statements are not really call expressions as far as the AST is concerned, so it's a bit hard to say whether there is a right or wrong answer for where the l- and r-paren locations are for the call.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157296/new/

https://reviews.llvm.org/D157296