[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.

[Richard Trieu via Phabricator via cfe-commits]

rtrieu added inline comments.


================
Comment at: lib/Sema/SemaStmt.cpp:2953
+                                  ExprResult &Res)
+{
+  ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
----------------
Opening brace should follow the closing paren on previous line.


================
Comment at: lib/Sema/SemaStmt.cpp:2963
+  InitializationSequence Seq(S, Entity, Kind, InitExpr);
+  if (Seq) {
+    for (const InitializationSequence::Step &Step : Seq.steps()) {
----------------
Use early exit here:


```
if (!Seq)
  return;

// rest of function
```


================
Comment at: lib/Sema/SemaStmt.cpp:2965-2966
+    for (const InitializationSequence::Step &Step : Seq.steps()) {
+      if (!(Step.Kind == InitializationSequence::SK_ConstructorInitialization ||
+            Step.Kind == InitializationSequence::SK_UserConversion))
+        continue;
----------------
Since you're simplifying the condition here, bring the not operator inside the parentheses.


```
if (Step.Kind != ...  && Step.Kind != ...)
```


================
Comment at: lib/Sema/SemaStmt.cpp:2970
+      FunctionDecl *FD = Step.Function.Function;
+      if (isa<CXXConstructorDecl>(FD)) {
+        // C++14 [class.copy]p32:
----------------
Use early exit here:


```
if (!isa<CXXConstructorDecl>(FD)
  continue;

// old if-block code
```


================
Comment at: lib/Sema/SemaStmt.cpp:2999-3000
-        // expression node to persist.
-        Value = ImplicitCastExpr::Create(Context, Value->getType(), CK_NoOp,
-                                         Value, nullptr, VK_XValue);
 
----------------
At this point, the variable Value is updated.  Value is scoped to this function, and used again after this code.  In your change, there is a new Value variable in the static function.  Only that variable is updated and not this one, making this a change in behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D43898