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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
