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

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 21 15:23:40 PST 2022


dexonsmith added a comment.

LGTM too; just wanted to share an optional suggestion.



================
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 {
----------------
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.


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:1413
 
-  if (auto *FT = dyn_cast<FunctionType>(PTy->getPointerElementType()))
+  Type *ElemTy = PTy->getNonOpaquePointerElementType();
+  if (auto *FT = dyn_cast<FunctionType>(ElemTy))
----------------
E.g., this could have:
```
lang=c++
// NOTE: getNonOpaquePointerElementType() call unreachable with opaque
// pointers.


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

https://reviews.llvm.org/D117870



More information about the llvm-commits mailing list