[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:2480
+                         SuppressResultRetain);
   }
 
----------------
rjmccall wrote:
> This switch is just checking what you already computed as `SuppressResultRetain`.  Please just assert in the second case that the qualifier is `OCL_Weak`.
> 
> Also, please stay consistent with the surrounding capitalization of local variables.
Not entirely sure I understand the comment about switch correctly. Will change the code according to my understanding.


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:2527
+      return TryEmitResult(CGF.EmitScalarExpr(e),
+                           !shouldRetainObjCLifetime(type.getObjCLifetime()));
+  }
----------------
rjmccall wrote:
> Can we test constant-evaluability directly instead of only applying this when the declaration isn't otherwise used?
We can use `IsVariableAConstantExpression` but it requires removing const from `Decl`.

Another way to test constant-evaluability is `tryEmitAsConstant`. It still requires removing const qualifier but overall it is a good approach. It captures the intention well and implementation-wise it is what `CGF.EmitScalarExpr` is doing anyway. I'll try it and will show how it looks like.


https://reviews.llvm.org/D53674