Quuxplusone marked 4 inline comments as done.
Quuxplusone added inline comments.

Comment at: lib/Sema/SemaStmt.cpp:2970
+      FunctionDecl *FD = Step.Function.Function;
+      if (isa<CXXConstructorDecl>(FD)) {
+        // C++14 [class.copy]p32:
rtrieu wrote:
> Use early exit here:
> ```
> if (!isa<CXXConstructorDecl>(FD)
>   continue;
> // old if-block code
> ```
I'd prefer not to do this, since D43322 is going to change this code into "if 
isa-red-fish... else if isa-blue-fish...". Therefore I think it makes sense to 
keep this intermediate stage as "if isa-red-fish...", rather than changing it 
into "if not-isa-red-fish continue... otherwise..."

If you really want this, I can change it; but it's just going to change back in 
D43322, and the goal of this patch was to make D43322 smaller.

Comment at: lib/Sema/SemaStmt.cpp:2999-3000
-        // expression node to persist.
-        Value = ImplicitCastExpr::Create(Context, Value->getType(), CK_NoOp,
-                                         Value, nullptr, VK_XValue);
rtrieu wrote:
> 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.
Good catch! I've addressed this now by making the parameter `Expr *&Value`; but 
I'd be open to better approaches. Particularly because I still don't know what 
to do about the "unnecessary promoting `Value` to the heap" that will happen in 

  rC Clang


cfe-commits mailing list

Reply via email to