[PATCH] D117870: [OpaquePtrs] Add getNonOpaquePointerElementType() method

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/include/llvm/IR/Type.h:375-378
+  /// Only use this method in code that is not reachable with opaque pointers,
+  /// or part of deprecated methods that will be removed as part of the opaque
+  /// pointers transition.
+  Type *getNonOpaquePointerElementType() const {
----------------
dblaikie wrote:
> dexonsmith wrote:
> > Given where this should be used, should all uses be required to be documented with a FIXME/TODO/SOMETHING to indicate which of the two cases it is?
> > 
> > I don't feel strongly (up to you), but it might make it easier to audit them.
> Yeah, or possibly have two different function names to self-document those two cases.
My 2c here is that it doesn't really matter why the method is used, just that the code will get deleted after the migration.

I think it's fairly obvious in most cases, because it's either directly in a deprecated API, or shortly after an isOpaque() check. A comment might be warranted if the check is far removed (e.g. the SROA case is a bit non-obvious, because the opaque pointer check happens in a different function.)


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

https://reviews.llvm.org/D117870