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