[PATCH] D58236: Make address space conversions a bit stricter.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/SemaCast.cpp:2306
+                     SrcPPointee.getAddressSpace()) ||
+          !DestPPtr->isAddressSpaceOverlapping(*SrcPPtr)) {
+        Self.Diag(OpRange.getBegin(),
----------------
This should `if (Nested ? DestPPointee.getAddressSpace() != SrcPPointee.getAddressSpace() : !DestPPtr->isAddressSpaceOverlapping(*SrcPtr))`, I think.


================
Comment at: lib/Sema/SemaExpr.cpp:7706
   if (!lhq.compatiblyIncludes(rhq)) {
     // Treat address-space mismatches as fatal.  TODO: address subspaces
     if (!lhq.isAddressSpaceSupersetOf(rhq))
----------------
The TODO here is fixed as much as anything else is.


================
Comment at: lib/Sema/SemaExpr.cpp:7781
       do {
+        // Inconsistent address spaces at this point is invalid, even if the
+        // address spaces would be compatible.
----------------
Please extract variables for `cast<PointerType>(lhptee)->getPointeeType()` (and for `rhptee`).


================
Comment at: lib/Sema/SemaExpr.cpp:14206
+        // XXX: Should this be a different variation of the error, like
+        // 'changes address space in nested pointer qualifiers'?
+        DiagKind = diag::err_typecheck_incompatible_address_space;
----------------
Yeah, I think that would be more straightforward.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58236/new/

https://reviews.llvm.org/D58236