[PATCH] D47067: Update NRVO logic to support early return

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Some minor nits. I would also like to see a test for the unoptimized (-O0) IR generated by Clang for the case where there are two NRVO variables in the same function. Otherwise, this looks good to go!



================
Comment at: lib/Sema/Scope.cpp:122-123
+void Scope::setNRVOCandidate(VarDecl *Candidate) {
+  for (auto* D : DeclsInScope) {
+    VarDecl* VD = dyn_cast_or_null<VarDecl>(D);
+    if (VD && VD != Candidate && VD->isNRVOCandidate())
----------------
`*` on the right, please. Also `auto` -> `Decl` would be clearer and no longer. Is `dyn_cast_or_null` really necessary? (Can `DeclsInScope` contain `nullptr`?) I would expect that just `dyn_cast` would suffice.


================
Comment at: lib/Sema/SemaDecl.cpp:12619-12620
 void Sema::computeNRVO(Stmt *Body, FunctionScopeInfo *Scope) {
-  ReturnStmt **Returns = Scope->Returns.data();
+  for (ReturnStmt* Return : Scope->Returns) {
+    const VarDecl* Candidate = Return->getNRVOCandidate();
+    if (!Candidate)
----------------
`*` on the right, please.


Repository:
  rC Clang

https://reviews.llvm.org/D47067