ilya-biryukov added inline comments.
================ Comment at: clang/lib/Serialization/ASTReaderStmt.cpp:2179 + for (unsigned I = 0; I < E->NumExprs; I++) + E->getTrailingObjects<Expr *>()[I] = Record.readSubExpr(); +} ---------------- ayzhao wrote: > ilya-biryukov wrote: > > FYI: I think this is where the crash comes from. > > We should allocate the trailing objects first. > > E.g. see how `PragmaCommentDecl::CreateDeserialized` does this. > This sounds like it could be the solution - thanks for looking at it! > > Currently, I'm working on the refactor that shafik@ suggested, which was to > inherit from `InitListExpr`. Hopefully, that refactor will fix this issue as > `InitListExpr` stores it's subexpressions in a class member instead of using > `llvm::TrailingObjects`. Are we trying to share code between two implementations? If so, I suggest to consider alternatives, e.g. creating a new base class and inheriting both `InitListExpr` and `CXXParentInitListExpr` to share the common code. Inheriting `CXXParentInitListExpr` from `InitListExpr` breaks [Liskov substitution principle](https://en.wikipedia.org/wiki/Liskov_substitution_principle) and will likely to lead to bugs that are hard to chase. `InitListExpr` is widely used and means`{}`. `CXXParenInitListExpr` is not `{}` and we do not know in advance which code is going to work for both and which code is only valid for `{}`. Reviewing all callsites that use `InitListExpr` does not seem plausible. Note that in addition to Clang, there are also uses in ast-matchers and in clang-tidy checks (quite a few of those are downstream). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129531/new/ https://reviews.llvm.org/D129531 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
