[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

It seems to me that the problem here is that `DeclMustBeEmitted` is not safe to call in the middle of deserialization if anything partially-deserialized might be reachable from the declaration we're querying, and yet we're currently calling it in that case. I don't see how this patch actually addresses that problem, though: if you build this testcase as a module instead of a PCH, won't it still fail in the same way that it does now?

I think we probably need to track all the declarations we deserialize in a top-level deserialization,  and only check whether they're interesting when we finish recursive deserialization (somewhere around `ASTReader::FinishedDeserializing`). For the update record case, this will need some care in order to retain the property that we only pass a declaration to the consumer once, but I think we can make that work by observing that it should generally be safe to call `DeclMustBeEmitted` on a declaration that we didn't create in this deserialization step, and when we're loading update records we know whether that's the case.


https://reviews.llvm.org/D29753