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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
