[PATCH] D49294: Sema: Fix explicit address space cast in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/SemaOverload.cpp:3150
+      !getLangOpts().OpenCLCPlusPlus)
+    return false;
+
----------------
yaxunl wrote:
> rjmccall wrote:
> > yaxunl wrote:
> > > rjmccall wrote:
> > > > It's not really OpenCL C++ that's special here, it's the possibility of promotions between address spaces.
> > > For OpenCL C++, there is language rule about address space promotion.
> > > 
> > > For other languages, there is no generic rule about adderss space promotion, since the meaning of address space and their relations are target dependent. Do we want to introduce a target hook Target::canPromote(AddressSpace Src, AddressSpace Dest, LanguageOptions LangOpts) to represent this? Or do we just assume a simple rule, that is, all address space can be promoted to default address space 0, otherwise it is not allowed?
> > A target-specific hook for handling target-specific address spaces makes sense to me.  I don't think there's a generic rule allowing promotion into the default AS.
> I checked the current logic about address space promotions. There are several functions Type::Qualifier::isAddressSpaceSupersetOf, Type:;Qualifier::compatiblyIncludes, PointerType::isAddressSpaceOverlapping which checks address space compatibility. However they are only based on language rules and do not depend on targets. Since only OpenCL defined language rules regarding address spaces, address space promotion is only allowed for OpenCL.
> 
> To allow target specific address space promotion, these functions need an extra ASTContext parameter to allow them access LanguageOptions and TargetInfo. I am not sure if this makes things overly complicated.
> 
> I would expect target specific address space casts in C++ to be rare cases since it is not part of language standard. In most cases users would just use generic pointer. When they do use explicit address space cast, I expect they understand what they are doing. Then clang just do not do any target-specific promotion and treat it as reinterpret cast. Promotion is only allowed by language rules e.g. those defined by OpenCL.
> 
Well, address spaces in "pure" C/C++ are specified by ISO/IEC TR 18037, the Embedded C specification, which does cover the possibility of target-specific overlapping address spaces.  Since we don't support those yet, that's fine, I'm not going to ask you to add that infrastructure; but I do think you should at least handle the language-specific overlap rules here, and in particular you should handle them by calling one of those functions you mentioned, so that if someone *does* want to implement target-specific overlapping address spaces, it's obvious that this is one of the places that needs to be fixed and tested.


https://reviews.llvm.org/D49294