ebevhan added inline comments.
================ Comment at: lib/Sema/SemaCast.cpp:2224 + } else if (IsLValueCast) { Kind = CK_LValueBitCast; } else if (DestType->isObjCObjectPointerType()) { ---------------- Anastasia wrote: > ebevhan wrote: > > 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? > > > > What if we have the reference cast case with a different address space like > > this? Doesn't the IsLValueCast check need to be first? > > Ok, let me see if I understand you. I changed `__generic` -> `__global` since > it's invalid and the error is produced as follows: > test.cl:7:21: error: reinterpret_cast from 'int' to '__global int &' is not > allowed > __global int& ref = reinterpret_cast<__global int&>(x); > > Is this not what we are expecting here? My last sentence could have been a bit clearer. Yes, for the `__private`->`__global` case, it should error. But I was thinking more of the case where the AS conversion is valid, like `__private`->`__generic`. Then we will do the AS conversion, but we should have done both an AS conversion and an `LValueBitCast`, because we need to both convert the 'pointer' and also dereference it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58346/new/ https://reviews.llvm.org/D58346 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits