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

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

I think the solution to a lot of diagnostic and behavior issues with address spaces is to remove predication on OpenCL. It's a bit odd to have a language feature that is enabled out of the box regardless of langoptions (address spaces) but won't actually work properly unless you're building OpenCL.



================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6943
+def err_typecheck_incompatible_conditional_pointer_operands : Error<
+  "unable to find common type between %0 and %1 for conditional operation">;
 
----------------
This error is very similar to the one in my first comment, `conditional operator with the second and third operands of type ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *') which are pointers to non-overlapping address spaces`. It would normally be emitted at 6472, but won't be since OpenCL isn't enabled.


================
Comment at: test/Sema/conditional-expr.c:78
+                       // expected-error at -1{{converting '__attribute__((address_space(2))) int *' to type 'void *' changes address space of pointer}}
+                       // expected-error at -2{{converting '__attribute__((address_space(3))) int *' to type 'void *' changes address space of pointer}}
 
----------------
rjmccall wrote:
> leonardchan wrote:
> > rjmccall wrote:
> > > Also, these diagnostics seem wrong.  Where is `void *` coming from?
> > When dumping the AST this is what the resulting type is for the conditional expression already is if the operands are 2 pointers with different address spaces.
> > 
> > According to this comment, the reason seems to be because this is what GCC does:
> > 
> > ```
> >  6512     // In this situation, we assume void* type. No especially good
> >  6513     // reason, but this is what gcc does, and we do have to pick
> >  6514     // to get a consistent AST.
> > ```
> That makes sense in general, but in this case it's not a great diagnostic; we should just emit an error when trying to pick a common type.
Is it possible that you are getting `void *` because we aren't running the qualifier removal at 6495? I don't think I've ever seen spurious `void *`'s show up in our downstream diagnostics.


Repository:
  rC Clang

https://reviews.llvm.org/D50278