[PATCH] D105321: [Bitcode][OpaquePtr] Remove usages of PointerType's getElementType()

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Ah, so this is intended to be an NFC patch - a different way of implementing the same functionality in the bitcode reader? A way that doesn't rely on PointerType having a pointee type present?

Seems pretty reasonable to me.



================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:5055-5056
+        PointeeTy = getElementType(Ptr->getType());
+        if (!PointeeTy)
+          return error("Missing pointee type");
+      }
----------------
Is this reachable? If so, it should be tested. If not, it should be an assertion, or omitted?

I think the original code with only the direct replacement of "X->getElementType()" -> "getElementType(X)" would be OK? Because we know by construction that it can't return null here? (if it'd be helpful, could have a separate `getElementType` that asserts (rather than returning null) and one (if needed called `getElementTypeOrNull` which does the returning null thing?)

But I think most of the getElementType queries will be the non-null kind, so maybe there's no need to have both, and the existing one you're adding here could be changed to assert, rather than returning null?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105321