[PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator

[Xiuli PAN via cfe-commits]

pxli168 added inline comments.

================
Comment at: lib/AST/ASTContext.cpp:7613
@@ +7612,3 @@
+    if (getLangOpts().OpenCL) {
+      if (LHS.getUnqualifiedType() != RHS.getUnqualifiedType() ||
+          LQuals.getCVRQualifiers() != RQuals.getCVRQualifiers())
----------------
Anastasia wrote:
> Do you think we should check the types here? I was thinking we should do the check exactly as below apart from AS specific part.
I think the mergeType function it very complex, too. It seems to check type can be merged recursively later.

================
Comment at: lib/Sema/SemaExpr.cpp:6168
@@ -6168,3 +6167,3 @@
   QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);
 
   if (CompositeTy.isNull()) {
----------------
Anastasia wrote:
> Could we instead add a comment explaining different cases with AS we can have here i.e. 1(a-c)&2(a-c)!
> 
> And may be we could refer to each case by adding comments in code below.
Good idea.

================
Comment at: lib/Sema/SemaExpr.cpp:6222-6227
@@ -6188,1 +6221,8 @@
+    auto ResultAddrSpace = ResultTy.getQualifiers().getAddressSpace();
+    LHSCastKind = lhQual.getAddressSpace() == ResultAddrSpace
+                      ? CK_BitCast
+                      : CK_AddressSpaceConversion;
+    RHSCastKind = rhQual.getAddressSpace() == ResultAddrSpace
+                      ? CK_BitCast
+                      : CK_AddressSpaceConversion;
     ResultTy = S.Context.getPointerType(ResultTy);
----------------
Anastasia wrote:
> pxli168 wrote:
> > What will mergetypes return?
> > It seems the LHS and RHS are compatibel here, and may be they did not need bitcast?
> I think we always need a cast here, because types are not exactly the same at this point!
I tried to figure out what mergetypes will return and find it seems to have logic far more complex than this.


Repository:
  rL LLVM

http://reviews.llvm.org/D17412