[OpenCL][PR48896] Fix address space in binding of initializer lists to references

[Anastasia]

Summary

This patch prevents materializing temporaries in the address space of the references they are bind to. The temporaries should always be in the same address space - private for OpenCL.

This has been fixed earlier for the initialization of references with regular literals in https://reviews.llvm.org/D58634 but the initialization list followed slightly different code path.

@ebevhan I only now noticed your comment on the old review and I agree this doesn't look right. I will prepare another patch, just need to create a reproducer first. Sorry about the delay.

[Anastasia]

• Added a test case.

[rjmccall]

clang/lib/Sema/SemaInit.cpp

4297

Should we be rejecting this path immediately if the address space in T1 can't be converted to from the address space of temporaries?

4308

This should only be VK_XValue if we're binding a r-value reference, I think.

[Anastasia]

Added address space compatibility check and improved testing.

[Anastasia]

clang/lib/Sema/SemaInit.cpp

4297

I agree. I have updated the patch and added more test cases.

4308

Yes, I think so too.

[rjmccall]

Thanks, LGTM with the value-kind fix.

clang/lib/Sema/SemaInit.cpp

4308

Did you mean to fix this to conditionally use VK_LValue?

[Anastasia]

Fixed value kind.

clang/lib/Sema/SemaInit.cpp

4308

Sure, I have added it now. Sorry I got confused somehow.

[rjmccall]

Thanks, LGTM.

I think idiomatically we normally just use (void) instead of spelling it out with static_cast, since there's only one possible meaning for a cast to void. I don't care deeply about it; if you make that change, feel free to make it as part of committing.

[Anastasia]

In D95608#2537316, @rjmccall wrote:

Thanks, LGTM.

I think idiomatically we normally just use (void) instead of spelling it out with static_cast, since there's only one possible meaning for a cast to void. I don't care deeply about it; if you make that change, feel free to make it as part of committing.

Thanks, sure I will stick to the existing practices and update this. Thanks!

clang/lib/Sema/SemaInit.cpp

     if (Sequence.step_begin() != Sequence.step_end())
       Sequence.RewrapReferenceInitList(cv1T1, InitList);
   }
-
+  // Ignore address space of reference type at this point and perform address
+  // space conversion after the reference binding step.
+  QualType cv1T1IgnoreAS =
+      T1Quals.hasAddressSpace()
+          ? S.Context.getQualifiedType(T1, T1Quals.withoutAddressSpace())
+          : cv1T1;
   // Not reference-related. Create a temporary and bind to that.
-  InitializedEntity TempEntity = InitializedEntity::InitializeTemporary(cv1T1);
+  InitializedEntity TempEntity = InitializedEntity::InitializeTemporary(cv1T1IgnoreAS);
 
   TryListInitialization(S, TempEntity, Kind, InitList, Sequence,
                         TreatUnavailableAsInvalid);
   if (Sequence) {
     if (DestType->isRValueReferenceType() ||
-        (T1Quals.hasConst() && !T1Quals.hasVolatile()))
-      Sequence.AddReferenceBindingStep(cv1T1, /*BindingTemporary=*/true);
+        (T1Quals.hasConst() && !T1Quals.hasVolatile()))//TODO: I am guessing we need the same check address space check here too?
+      Sequence.AddReferenceBindingStep(cv1T1IgnoreAS, /*BindingTemporary=*/true);
     else
       Sequence.SetFailed(
           InitializationSequence::FK_NonConstLValueReferenceBindingToTemporary);
+    if (T1Quals.hasAddressSpace())
+      Sequence.AddQualificationConversionStep(cv1T1, !DestType->isRValueReferenceType() ? VK_LValue
+                                                               : VK_XValue);
   }
 }
 

   //        than, cv2; otherwise, the program is ill-formed.
   unsigned T1CVRQuals = T1Quals.getCVRQualifiers();
   unsigned T2CVRQuals = T2Quals.getCVRQualifiers();
-  if ((RefRelationship == Sema::Ref_Related &&
-       (T1CVRQuals | T2CVRQuals) != T1CVRQuals) ||
-      !T1Quals.isAddressSpaceSupersetOf(T2Quals)) {
+  if (RefRelationship == Sema::Ref_Related &&
+       ((T1CVRQuals | T2CVRQuals) != T1CVRQuals ||
+      !T1Quals.isAddressSpaceSupersetOf(T2Quals))) {
     Sequence.SetFailed(InitializationSequence::FK_ReferenceInitDropsQualifiers);
     return;
   }