[PATCH] D103429: [OpaquePtr] Create API to make a copy of a PointerType with some address space

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/IR/DerivedTypes.h:667
+  /// transition.
+  static PointerType *getWithSamePointeeType(PointerType *PT,
+                                             unsigned AddressSpace) {
----------------
aeubanks wrote:
> dblaikie wrote:
> > Maybe the "with same pointee type" isn't the high priority aspect of this API? More "get with different address space" and it's assumed the rest of the type remains the same? (opaque to opaque, typed to typed)
> > 
> > Maybe "getWithAddressSpace" or something like that?
> There are already a couple `PointerType::get()` methods which construct a pointer type. This one is different in that it's sorta cloning an existing PointerType. I want to make that super clear via the name somehow. This could be changed to a non-static method to be clearer? IMO Something like `getWithAddressSpace()` is too similar to the existing methods.
> 
> And this should be removed after the opaque pointer transition is over.
Yeah, happy for this to go away after the opaque pointer transition - when nothing significant from the old pointer type would be used to make the new pointer type anyway (since the only interesting property of the opaque pointer is its address space).

I don't too much mind it being a non-static member - it is different in that it's derived from the current pointer type so it can readily be a non-static member function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103429