[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Josh Haberman via Phabricator via cfe-commits]

haberman added inline comments.


================
Comment at: clang/lib/Sema/SemaStmt.cpp:605-609
+  while (!isa<CallExpr>(Ex)) {
+    auto *PE = cast<ParenExpr>(Ex);
+    Ex = IgnoreImplicitAsWritten(PE->getSubExpr());
+    PE->setSubExpr(Ex);
+  }
----------------
rsmith wrote:
> This loop is problematic: it's generally not safe to modify an expression that is used as a subexpression of another expression. (Modifying the `ReturnStmt` is, by contrast, much less problematic because the properties of a statement have less complex dependencies on the properties of its subexpressions.) In particular, if there were any implicit conversions here that changed the type or value category or similar, the enclosing parentheses would have the wrong type / value category / similar. Also there are possibilities here other than `CallExpr` and `ParenExpr`, such as anything else that we consider to be "parentheses" (such as a `GenericSelectionExpr`).
> 
> But I think this loop should never be necessary, because all implicit conversions should always be on the outside of the parentheses. Do you have a testcase that needs it?
I removed it and my test cases still pass. I'm glad to know this isn't necessary: I was coding defensively because I didn't know that I could count on this invariant:

> all implicit conversions should always be on the outside of the parentheses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517