riccibruno added a comment. In D82940#2125939 <https://reviews.llvm.org/D82940#2125939>, @martong wrote:
> In D82940#2125183 <https://reviews.llvm.org/D82940#2125183>, @riccibruno > wrote: > > > > I'm not certain I understand, but I'm removing my LG while we figure it > > > out. Can you expound a bit further? > > > > Take a look at the AST dump of a small modification of the reduced test > > case which doesn't trigger the assertion: > > > > void should_not_crash_with_switch_in_lambda() { > > switch (1) > > default:; > > auto f = [] { > > switch (1) > > default:; > > }; > > } > > > > > > Before the serialization of `LambdaExpr` for the `LambdaExpr *E`, we have > > `E->getBody() == E->getCallOperator()->getBody()`. > > After serialization this is not true anymore because we are writing and > > reading the body directly when visiting the `LambdaExpr` > > (the captures have the same problem). > > > The same is true in case of the simplest lambda example: > > void foo() { // line 11 > []{}; > } > > > The AST that we got from the Parser has only one CompoundStmt node, but > referenced in the CXXMethodDecl and added as a direct child to LambdaExpr: > > `-LambdaExpr 0x5643235372c8 <line:12:5, col:8> > |-CXXRecordDecl 0x564323536c98 <col:5> col:5 implicit class definition > | |-CXXMethodDecl 0x564323536dd8 <col:6, col:8> col:5 constexpr > operator() 'auto () const -> void' inline > | | `-CompoundStmt 0x564323536e88 <col:7, col:8> > `-CompoundStmt 0x564323536e88 <col:7, col:8> > > > After serialization and deserialization, the CompoundStmt is just copied, > notice the different pointer values: > > `-LambdaExpr 0x55a49f35f830 <line:12:5, col:8> > |-CXXRecordDecl 0x55a49f35f860 <col:5> col:5 imported implicit > <undeserialized declarations> class definition > | |-CXXMethodDecl 0x55a49f35fac8 <col:6, col:8> col:5 imported constexpr > operator() 'auto () const -> void' inline > | | `-CompoundStmt 0x55a49f360110 <col:7, col:8> > `-CompoundStmt 0x55a49f35f820 <col:7, col:8> > > > This suggests that we should prepare the ASTReader/ASTWriter to be able to > handle references to Stmts similarly as it can already handle references to > Decls (see `ASTWriter::DeclIDs`). > Does this sound as a good direction, should I move forward this way? Yes, I think that my patch just traded-off a bug for another bug. What about the following instead: Initially zero the `Stmt *` for the body, then when the body is needed (either with `getBody` or with `children`) populate it if needed from the call operator. This is what was done before with `getBody`. This preserve the benefit of having a lazily deserialized body while being a simple fix. Thoughts? Am I missing something? If not I can cook-up a patch either tonight or tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82940/new/ https://reviews.llvm.org/D82940 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
