[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 159405.
leonardchan marked an inline comment as done.
leonardchan added a comment.

- Replaced instances of a `pointer type mismatch` warning involving 2 conditional operands with different address spaces with a new error specifically for this situation.


Repository:
  rC Clang

https://reviews.llvm.org/D50278

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/address_spaces.c
  test/Sema/conditional-expr.c


Index: test/Sema/conditional-expr.c
===================================================================
--- test/Sema/conditional-expr.c
+++ test/Sema/conditional-expr.c
@@ -73,10 +73,12 @@
 
   int __attribute__((address_space(2))) *adr2;
   int __attribute__((address_space(3))) *adr3;
-  test0 ? adr2 : adr3; // expected-warning {{pointer type mismatch}} expected-warning {{expression result unused}}
+  test0 ? adr2 : adr3; // expected-warning {{expression result unused}}
+                       // expected-error at -1{{unable to find common type between '__attribute__((address_space(2))) int *' and '__attribute__((address_space(3))) int *' for conditional operation}}
 
   // Make sure address-space mask ends up in the result type
-  (test0 ? (test0 ? adr2 : adr2) : nonconst_int); // expected-warning {{pointer type mismatch}} expected-warning {{expression result unused}}
+  (test0 ? (test0 ? adr2 : adr2) : nonconst_int); // expected-warning {{expression result unused}}
+                                                  // expected-error at -1{{unable to find common type between '__attribute__((address_space(2))) int *' and 'int *' for conditional operation}}
 }
 
 int Postgresql() {
Index: test/Sema/address_spaces.c
===================================================================
--- test/Sema/address_spaces.c
+++ test/Sema/address_spaces.c
@@ -71,5 +71,5 @@
 
 // Clang extension doesn't forbid operations on pointers to different address spaces.
 char* cmp(_AS1 char *x,  _AS2 char *y) {
-  return x < y ? x : y; // expected-warning {{pointer type mismatch ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *')}}
+  return x < y ? x : y; // expected-error{{unable to find common type between '__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *' for conditional operation}}
 }
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -6517,16 +6517,29 @@
         S.Context.getAddrSpaceQualType(S.Context.VoidTy, ResultAddrSpace));
     LHS = S.ImpCastExprToType(LHS.get(), incompatTy, LHSCastKind);
     RHS = S.ImpCastExprToType(RHS.get(), incompatTy, RHSCastKind);
-    // FIXME: For OpenCL the warning emission and cast to void* leaves a room
-    // for casts between types with incompatible address space qualifiers.
-    // For the following code the compiler produces casts between global and
-    // local address spaces of the corresponded innermost pointees:
-    // local int *global *a;
-    // global int *global *b;
-    // a = (0 ? a : b); // see C99 6.5.16.1.p1.
-    S.Diag(Loc, diag::ext_typecheck_cond_incompatible_pointers)
-        << LHSTy << RHSTy << LHS.get()->getSourceRange()
-        << RHS.get()->getSourceRange();
+
+    bool HasDifferingLAddrSpace = LAddrSpace != ResultAddrSpace;
+    bool HasDifferingRAddrSpace = RAddrSpace != ResultAddrSpace;
+
+    if (S.getLangOpts().OpenCL ||
+        !(HasDifferingLAddrSpace || HasDifferingRAddrSpace)) {
+      // FIXME: For OpenCL the warning emission and cast to void* leaves a room
+      // for casts between types with incompatible address space qualifiers.
+      // For the following code the compiler produces casts between global and
+      // local address spaces of the corresponded innermost pointees:
+      // local int *global *a;
+      // global int *global *b;
+      // a = (0 ? a : b); // see C99 6.5.16.1.p1.
+      S.Diag(Loc, diag::ext_typecheck_cond_incompatible_pointers)
+          << LHSTy << RHSTy << LHS.get()->getSourceRange()
+          << RHS.get()->getSourceRange();
+    } else {
+      // If the addresse spacesare different, we do not allow a bitcast.
+      S.Diag(Loc, diag::err_typecheck_incompatible_conditional_pointer_operands)
+          << LHSTy << RHSTy << Sema::AA_Converting
+          << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+    }
+
     return incompatTy;
   }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6939,6 +6939,8 @@
   " changes retain/release properties of pointer">;
 def err_typecheck_comparison_of_distinct_blocks : Error<
   "comparison of distinct block types%diff{ ($ and $)|}0,1">;
+def err_typecheck_incompatible_conditional_pointer_operands : Error<
+  "unable to find common type between %0 and %1 for conditional operation">;
 
 def err_typecheck_array_not_modifiable_lvalue : Error<
   "array type %0 is not assignable">;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50278.159405.patch
Type: text/x-patch
Size: 4654 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180806/d4aa45ff/attachment.bin>