[clang] [ASTReader] Remove assert in GetExternalDeclStmt (PR #143739)

[Henrik G. Olsson via cfe-commits]

hnrklssn wrote:

> I think we shouldn't remove the assertion. Your test passes with the removal of the assertion since the initializers are not complex. So it ends quickly. But if it is a complex initialization which triggers more deserialization, I feel it will be problematic.
> 
> I think the point is in `DeclMustBeEmitted`, this should be a "pure" method but it triggers deserialization.
> 
> I think, the proper solution may be:
> 
>     1. When we write a VarDecl, use a bit to record whether the var decl has an initialization with side effects.
> 
>     2. When we read a var decl with the above information, let's record it in a set in ASTReader.
> 
>     3. When we decide if a VarDecl needs to be emitted in `DeclMustBeEmitted`, let's lookup it in the above set.

Thanks for providing context! I'm not very familiar with this part of clang. I have a few clarifying questions:
`DeclMustBeEmitted` is part of `ASTContext` rather than  `ASTReader`, so unless I missed something it wouldn't have access to this set. We could check the "has initializer with side effect" set in `ASTReader` before the call to `DeclMustBeEmitted`, but unless we removed that piece of code from `DeclMustBeEmitted` it would still result in the initializer being deserialized, right? There are other callers that would not necessarily have an`ASTReader` in context, so I'm not sure how to go about that.

Actually, looking at the callers, one of them is `isRequiredDecl`, which is used to determine whether decls go into the `EagerlyDeserializedDecls` record. I'm not fully clear on how eagerly they are deserialized, but if the initializer is also eagerly deserialized, then maybe the `ASTReader` could skip calling `DeclMustBeEmitted` for `VarDecl`s that aren't fully deserialized, since they would have been in the `EagerlyDeserializedDecls` record if `DeclMustBeEmitted` returned true.

Another option would be to set some bits in `VarDecl::NonParmVarDeclBits` rather than creating a set in `ASTReader`.
WDYT?

https://github.com/llvm/llvm-project/pull/143739