Anastasia added a comment.
@svenvh We haven't looked into address spaces in details when adding this. I
wonder what behavior would make sense for new/delete? Do we plan to allow
new/delete in named address spaces or only in generic? It might be more helpful
to allow named address spaces since generic has not been intended for
allocations, but however generic makes things easier because it avoids
duplication and the concrete address space can always be cast to it. I think
`constant` doesn't make sense at all since nothing can be done with a readonly
chunk of memory?
Another aspect to consider - how will address spaces aligned with the concept
of overloading i.e.
- Declaring multiple new operators with different address spaces in the same
scope generates an error since overloading by the return type is not allowed.
- Declaring multiple delete operators differing by address spaces seems to fail
because C++ doesn't allow to overload it yet.
While the test in the review works, if we start using other than generic
address spaces it fails with various compile-time errors. For example, I get an
error if I try to assign to `local` address space pointer even if the
declaration of new has the address space too:
`class A {
public:
__local void* operator new(size_t);
};
__local A* a = new A;`
`error: assigning 'A *' to '__local A *' changes address space of pointer`
Perhaps this deserves a PR at least?
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15287
+
+ if (CanQual<PointerType> ExpectedPtrTy =
+ ExpectedResultType->getAs<PointerType>()) {
----------------
I think `getAs` returns a `PointerType*` that we could just use straight away
without the need for `CanQual`? but actually, you could just change to `auto*`
that would make things easier.
The same below.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15289
+ ExpectedResultType->getAs<PointerType>()) {
+ ExpectedResultType = SemaRef.Context.getCanonicalType(
+ RemoveAddressSpaceFromPtr(SemaRef, ExpectedPtrTy->getTypePtr()));
----------------
FYI I think `ExpectedResultType` is going to be in always in the canonical form
here by the way it is constructed so `getCanonicalType` should be redundant.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96178/new/
https://reviews.llvm.org/D96178
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
