[PATCH] D103503: [OpaquePtr] Introduce option to force all pointers to be opaque pointers

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/test/Other/force-opaque-ptrs.ll:1
+; RUN: llvm-as --force-opaque-pointers < %s | llvm-dis | FileCheck %s
+
----------------
aeubanks wrote:
> dexonsmith wrote:
> > Can you explain why this test uses `--force-opaque-pointers`? I feel like this test would be more clear if the textual IR actually matched the IR under test (i.e., if it used `ptr`)... also, then the textual IR won't need to be updated later when typed pointers are removed. But maybe there's some limitation you're working around?
> I assumed that at some point we were going to update all tests from `type *` to `ptr`. I think there was a discussion long ago about this but it was never really resolved. We should probably raise this question on llvm-dev.
Hmm, I might just end up adding more confusion to this thread, but I'm not sure how your answer, @aeubanks applies to @dexonsmith's question - perhaps you could rephrase?

My understanding of @dexonsmith's question, combined with what @aeubanks has mentioned elsewhere in the review:

`--force-opaque-pointers` is needed because this test is intended to test that functionality/flag, including the fix in the bitcode reader for the types of functions.

But it might be that this could be separated - the flag could be added and tested purely with IR reading/writing, showing a pointer type going from typed to opaque through the IR roundtrip?

Then separately a commit that shows the bitcode change/fix?

(not sure that explains all of @dexonsmith's questions)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103503