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

[vsapsai]

Summary

Failed assertion is

Assertion failed: ((ND->isUsed(false) || !isa<VarDecl>(ND) || !E->getLocation().isValid()) && "Should not use decl without marking it used!"), function EmitDeclRefLValue, file llvm-project/clang/lib/CodeGen/CGExpr.cpp, line 2437.

EmitDeclRefLValue mentions

A DeclRefExpr for a reference initialized by a constant expression can
appear without being odr-used. Directly emit the constant initializer.

The fix is in using the similar approach for non-references of
non-odr-used variables. EmitScalarExpr will try to emit constant if
possible and we can use resulting llvm::Value * without performing
EmitLValue.

rdar://problem/40650504

[rjmccall]

clang/lib/CodeGen/CGObjC.cpp

2480

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.

2527

Can we test constant-evaluability directly instead of only applying this when the declaration isn't otherwise used?

[vsapsai]

clang/lib/CodeGen/CGObjC.cpp

2480

Not entirely sure I understand the comment about switch correctly. Will change the code according to my understanding.

2527

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.

[vsapsai]

• Address review comments.

[vsapsai]

clang/lib/CodeGen/CGObjC.cpp

2527

If we go with tryEmitAsConstant, there is a separate review D53725 to use EmitConstant in more places.

[rjmccall]

clang/lib/CodeGen/CGObjC.cpp

2480

That's exactly what I was asking for, thank you.

[vsapsai]

• Rename EmitConstant to emitScalarConstant.
• Tweak comment to be explicitly about intended IR code, not about Obj-C++ code.

[vsapsai]

Exclude commits tracked in a different review.

[rjmccall]

LGTM.

[vsapsai]

Thanks for the review, John. I'll update the comment with TODO and commit.

clang/lib/CodeGen/CGObjC.cpp

 
 void CodeGenFunction::EmitObjCAutoreleasePoolCleanup(llvm::Value *Ptr) {
   if (CGM.getLangOpts().ObjCAutoRefCount)
     EHStack.pushCleanup<CallObjCAutoreleasePoolObject>(NormalCleanup, Ptr);
   else
     EHStack.pushCleanup<CallObjCMRRAutoreleasePoolObject>(NormalCleanup, Ptr);
 }
 
+static bool shouldRetainObjCLifetime(Qualifiers::ObjCLifetime Lifetime) {
+  switch (Lifetime) {
+  case Qualifiers::OCL_None:
+  case Qualifiers::OCL_ExplicitNone:
+  case Qualifiers::OCL_Strong:
+  case Qualifiers::OCL_Autoreleasing:
+    return true;
+
+  case Qualifiers::OCL_Weak:
+    return false;
+  }
+
+  llvm_unreachable("impossible lifetime!");
+}
+
 static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF,
                                                   LValue lvalue,
                                                   QualType type) {
+  bool SuppressResultRetain = !shouldRetainObjCLifetime(type.getObjCLifetime());
   switch (type.getObjCLifetime()) {
   case Qualifiers::OCL_None:
   case Qualifiers::OCL_ExplicitNone:
   case Qualifiers::OCL_Strong:
   case Qualifiers::OCL_Autoreleasing:
-    return TryEmitResult(CGF.EmitLoadOfLValue(lvalue,
-                                              SourceLocation()).getScalarVal(),
-                         false);
+    return TryEmitResult(
+        CGF.EmitLoadOfLValue(lvalue, SourceLocation()).getScalarVal(),
+        SuppressResultRetain);
 
   case Qualifiers::OCL_Weak:
     return TryEmitResult(CGF.EmitARCLoadWeakRetained(lvalue.getAddress()),
-                         true);
+                         SuppressResultRetain);
   }
 
   llvm_unreachable("impossible lifetime!");
 }
 
 static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF,
                                                   const Expr *e) {
   e = e->IgnoreParens();
   QualType type = e->getType();
static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF,
   // result of the call to objc_storeWeak instead of reloading.
   if (CGF.getLangOpts().CPlusPlus &&
       !type.isVolatileQualified() &&
       type.getObjCLifetime() == Qualifiers::OCL_Weak &&
       isa<BinaryOperator>(e) &&
       cast<BinaryOperator>(e)->getOpcode() == BO_Assign)
     return TryEmitResult(CGF.EmitScalarExpr(e), false);
 
+  // DeclRefExpr referencing non-odr-used variable might be constant-evaluated
+  // and in this case we don't have l-value. Emit code for scalar expression
+  // instead of emitting LValue and loading it.
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(e)) {
+    const ValueDecl *Decl = DRE->getDecl();
+    if (isa<VarDecl>(Decl) && !Decl->isUsed(false) &&
+        !Decl->getType()->isReferenceType())
+      return TryEmitResult(CGF.EmitScalarExpr(e),
+                           !shouldRetainObjCLifetime(type.getObjCLifetime()));
+  }
+
   return tryEmitARCRetainLoadOfScalar(CGF, CGF.EmitLValue(e), type);
 }
 
 typedef llvm::function_ref<llvm::Value *(CodeGenFunction &CGF,
                                          llvm::Value *value)>
   ValueTransform;
 
 /// Insert code immediately after a call.

clang/test/CodeGenObjCXX/arc-constexpr.mm

-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -o - -std=c++11 %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -fobjc-runtime-has-weak -o - -std=c++11 %s | FileCheck %s
 
 // CHECK: %[[TYPE:[a-z0-9]+]] = type opaque
 // CHECK: @[[CFSTRING:[a-z0-9_]+]] = private global %struct.__NSConstantString_tag
+@class NSString;
 
-// CHECK: define void @_Z5test1v
+// CHECK-LABEL: define void @_Z5test1v
 // CHECK:   %[[ALLOCA:[A-Z]+]] = alloca %[[TYPE]]*
 // CHECK:   %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]]
 // CHECK:   %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]*
 // CHECK:   store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[ALLOCA]]
 // CHECK:   %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[ALLOCA]]
 // CHECK:   call void @objc_storeStrong(i8** %[[V2]], i8* null)
-
-@class NSString;
-
 void test1() {
   constexpr NSString *S = @"abc";
 }
+
+// CHECK-LABEL: define void @_Z5test2v
+// CHECK:      %[[CONST:[a-zA-Z]+]] = alloca %[[TYPE]]*
+// CHECK:      %[[REF_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]*
+// CHECK:      %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]]
+// CHECK-NEXT: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]*
+// CHECK-NEXT: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[CONST]]
+// CHECK:      %[[V2:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]]
+// CHECK-NEXT: %[[V3:[0-9]+]] = bitcast i8* %[[V2]] to %[[TYPE]]*
+// CHECK-NEXT: store %[[TYPE]]* %[[V3]], %[[TYPE]]** %[[REF_CONST]]
+// CHECK:      %[[V4:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_CONST]]
+// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V4]], i8* null)
+// CHECK:      %[[V5:[0-9]+]] = bitcast %[[TYPE]]** %[[CONST]]
+// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V5]], i8* null)
+void test2() {
+  constexpr NSString *Const = @"abc";
+  // Initialize RefConst with Const initializer instead of reading from variable.
+  NSString* RefConst = Const;
+}
+
+// CHECK-LABEL: define void @_Z5test3v
+// CHECK:      %[[WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]*
+// CHECK:      %[[REF_WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]*
+// CHECK:      %[[V0:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]]
+// CHECK-NEXT: %[[V1:[0-9]+]] = call i8* @objc_initWeak(i8** %[[V0]], i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]]
+// CHECK:      store %[[TYPE]]* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] to %[[TYPE]]*), %[[TYPE]]** %[[REF_WEAK_CONST]]
+// CHECK:      %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_WEAK_CONST]]
+// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V2]], i8* null)
+// CHECK:      %[[V3:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]]
+// CHECK-NEXT: call void @objc_destroyWeak(i8** %[[V3]])
+void test3() {
+  __weak constexpr NSString *WeakConst = @"abc";
+  NSString* RefWeakConst = WeakConst;
+}