[OpaquePtrs] Deprecate PointerType::getElementType()

[nikic]

Summary

This deprecates PointerType::getElementType() in favor of Type::getPointerElementType(). The motivation is to make it more apparent when code accesses the pointer element type, because getElementType() may also also refer to at least ArrayType::getElementType() and VectorType::getElementType().

Note that this is only a partial change -- if the general idea is agreeable, I'm going to convert uses of PointerType::getElementType() first, and then commit the actual deprecation as a separate change lateron. The current revision is just to give a general impression.

Depends on D117870.

[aeubanks]

sounds good to me

[dexonsmith]

Generally makes sense to me; staging you propose sounds right too.

Wondering though, why "deprecated" instead of simple delete? Either one is liable to need a temporary revert in downstreams... I figure, either they'll care about warnings (maybe use -Werror) and'll need to revert, or they won't care and this'll go by without being fixed. But I'm not sure / don't feel strongly / etc.

[dblaikie]

In D117885#3262673, @dexonsmith wrote:

Generally makes sense to me; staging you propose sounds right too.

Wondering though, why "deprecated" instead of simple delete? Either one is liable to need a temporary revert in downstreams... I figure, either they'll care about warnings (maybe use -Werror) and'll need to revert, or they won't care and this'll go by without being fixed. But I'm not sure / don't feel strongly / etc.

Bit easier to disable the deprecation warning than revert the patch locally, perhaps. I think a deprecation's not a bad idea - and at least removing the function in a patch separately from the patch that removes all uses of it should make it easier for downstream folks.

(maybe even split this patch in two - one to add the new function, and another to deprecate the old one)

[nikic]

In D117885#3262673, @dexonsmith wrote:

Generally makes sense to me; staging you propose sounds right too.

Wondering though, why "deprecated" instead of simple delete? Either one is liable to need a temporary revert in downstreams... I figure, either they'll care about warnings (maybe use -Werror) and'll need to revert, or they won't care and this'll go by without being fixed. But I'm not sure / don't feel strongly / etc.

I'd like to get this landed before the LLVM 14 branching, and I think it would be nice if the method is only deprecated in the release, so people don't have to scramble getting this resolved as part of the upgrade.

[nikic]

Update more uses.

[dblaikie]

Sounds good - though please separate it - API migration (one or many patches, I don't much mind) and then a separate one to flag the function as deprecated. (make it easier to revert if needed, or for downstream users to revert/#ifdef out/whatever they need to do)

[dexonsmith]

In D117885#3263324, @nikic wrote:

In D117885#3262673, @dexonsmith wrote:

Generally makes sense to me; staging you propose sounds right too.

Wondering though, why "deprecated" instead of simple delete? Either one is liable to need a temporary revert in downstreams... I figure, either they'll care about warnings (maybe use -Werror) and'll need to revert, or they won't care and this'll go by without being fixed. But I'm not sure / don't feel strongly / etc.

I'd like to get this landed before the LLVM 14 branching, and I think it would be nice if the method is only deprecated in the release, so people don't have to scramble getting this resolved as part of the upgrade.

SG!

llvm/include/llvm/IR/DerivedTypes.h

public:
   /// PointerType (or opaque pointer is the input PointerType is opaque) and the
   /// given address space. This is only useful during the opaque pointer
   /// transition.
   /// TODO: remove after opaque pointer transition is complete.
   static PointerType *getWithSamePointeeType(PointerType *PT,
                                              unsigned AddressSpace) {
     if (PT->isOpaque())
       return get(PT->getContext(), AddressSpace);
-    return get(PT->getElementType(), AddressSpace);
+    return get(PT->PointeeTy, AddressSpace);
   }
 
+  [[deprecated("Pointer element types are deprecated. You can *temporarily* "
+               "use Type::getPointerElementType() instead")]]
   Type *getElementType() const {
     assert(!isOpaque() && "Attempting to get element type of opaque pointer");
     return PointeeTy;
   }
 
   bool isOpaque() const { return !PointeeTy; }
 
   /// Return true if the specified type is valid as a element type.