[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.


================
Comment at: lib/Sema/SemaCast.cpp:2224
+  } else if (IsLValueCast) {
     Kind = CK_LValueBitCast;
   } else if (DestType->isObjCObjectPointerType()) {
----------------
This might not be applicable to this patch, but just something I noticed.

So `reinterpret_cast` only operates on pointers when dealing with address spaces. What about something like
```
T a;
T& a_ref = reinterpret_cast<T&>(a);
```
`reinterpret_cast` on an lvalue like this is equivalent to `*reinterpret_cast<T*>(&a)`. So for AS code:
```
__private T x;
__generic T& ref = reinterpret_cast<__generic T&>(x);
```
This should work, since `*reinterpret_cast<__generic T*>(&x)` is valid, correct?

What if we have the reference cast case with a different address space like this? Doesn't the `IsLValueCast` check need to be first?



================
Comment at: lib/Sema/SemaCast.cpp:2309
+    auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType(
+        DestPointeeType.getCanonicalType());
+    return Self.Context.hasSameType(SrcPointeeTypeWithoutAS,
----------------
Maybe I'm mistaken, but won't getting the canonical type here drop qualifiers (like cv) in nested pointers? If so, an addrspace_cast might strip qualifiers by mistake.


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

https://reviews.llvm.org/D58346