[PATCH] D127728: [BitcodeReader] Allow reading pointer types from old IR

[Sebastian Neubauer via Phabricator via llvm-commits]

sebastian-ne marked an inline comment as done.
sebastian-ne added inline comments.


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:1337-1340
   // If we have a forward reference, the only possible case is when it is to a
   // named struct.  Just create a placeholder for now.
   return TypeList[ID] = createIdentifiedStructType(Context);
 }
----------------
jsilvanus wrote:
> The assumptions here seem to be based on the existing calls from within the parser. If we expose `getTypeByID` and `getContainedTypeID` to callers, these might no longer hold.
> 
> Maybe use an additional argument that flags external callbacks, and just return `nullptr` in that case?
I’m not completely sure in which case this could break something, but I think the callback added in this patch is fine anyway.
It only gets called on values that are already created in the IR Module, so I assume their type is completely known at this point.

I guess a case where getTypeByID can’t return anything meaningful is for cycles is structs, i.e.
```
struct A { B*; }
struct B { A*; }
```

The callbacks added here don’t get called on (potentially incomplete) types and even if they would, I think the current behavior of getTypeByID is the most sensible thing to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127728