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

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: lib/Sema/SemaExpr.cpp:6522
+    bool HasDifferingLAddrSpace = LAddrSpace != ResultAddrSpace;
+    bool HasDifferingRAddrSpace = RAddrSpace != ResultAddrSpace;
+
----------------
leonardchan wrote:
> rjmccall wrote:
> > I was going to tell you to use the predicate `Qualifiers::isAddressSpaceSupersetOf` here, but then I was looking at the uses of that, and I think the real fix is to just go into the implementation of `checkConditionalPointerCompatibility` and make the compatibility logic not OpenCL-specific.  The fast-path should just be whether the address spaces are different.
> > 
> > And it looks like this function has a bug where it always uses `LangAS::Default` outside of OpenCL even if the pointers are in the same address space.
> I'm not sure how the `LangAS::Default`, but removing all checks for OpenCL does the trick and prints an existing error relating to different address_spaces on conditional operands to replace the warning. Only 2 tests needed the change from the expected warning to expected error without having to change any OpenCL tests.
> 
> I also think the address_space comparison is already done with the `lhQual.isAddressSpaceSupersetOf` and `rhQual.isAddressSpaceSupersetOf`.
Er, you're right, of course, since `isAddressSpaceSupersetOf` is a non-strict ordering.  If that operation ever gets big enough that we don't want to inline the whole thing, we can at least make sure the fast-path is inlinable and then outline the complicated stuff.  We can also worry about that later.


Repository:
  rC Clang

https://reviews.llvm.org/D50278