rsmith added inline comments.
================ Comment at: include/clang/AST/Stmt.h:206 + bool PartOfExplicitCast : 1; + unsigned BasePathSize : 32 - 6 - 1 - NumExprBits; }; ---------------- lebedev.ri wrote: > rjmccall wrote: > > lebedev.ri wrote: > > > rjmccall wrote: > > > > This needs to be serialized. > > > Uhm, could you please explain what do you mean by 'serialized'? > > It needs to be preserved when writing an ICE into a PCH / module file. See > > the ASTWriter / ASTReader. > Aha. I did add handling there but it raises questions: > # This will silently break with different AST serialization versions. > I'm not sure how to handle it, since `VERSION_MINOR` isn't even read > back. > # //Does// this need a test? How to write one? > Like `./test/PCH/include-timestamp.cpp`, using `llvm-bcanalyzer`? Don't worry about breaking the serialization format. We do not maintain AST file format compatibility in general (neither between major releases nor between any two arbitrary SVN revisions). [We should probably maintain file format compatibility between patch releases, but I don't think that works right now because we check the full version number including the patch level when reading a file.] Please do add a test: what you need to do is create a PCH containing an implicit cast expression and then import that AST file and do anything to check that the value is preserved (such as inspecting the output of `-ast-dump`). Maybe you could add this to the existing `test/PCH/cxx_exprs.cpp` test, which already does most of what you want, but doesn't have any `FileCheck` tests on the `-ast-dump` output yet. ================ Comment at: lib/AST/ASTDumper.cpp:2122 + if (Node->getIsPartOfExplicitCast()) + OS << " PartOfExplicitCast"; } ---------------- Our predominant convention is to use lower_snake_case for such things in `-ast-dump` (though we're not exactly consistent on this...) ================ Comment at: lib/Sema/SemaCast.cpp:94-101 + void updatePartOfExplicitCastFlags(CastExpr *CE) { + // Walk down from the CE to the OrigSrcExpr, and mark all immediate + // ImplicitCastExpr's as being part of ExplicitCastExpr. The original CE + // (which is a ExplicitCastExpr), and the OrigSrcExpr are not touched. + while (OrigSrcExpr.get() != CE->getSubExpr() && + (CE = dyn_cast<ImplicitCastExpr>(CE->getSubExpr()))) + CE->setIsPartOfExplicitCast(true); ---------------- You don't need to track the `OrigSrcExpr` here. You can just recurse down through all the `ImplicitCastExpr`s (they're always all notionally part of the explicit cast). Repository: rC Clang https://reviews.llvm.org/D49508 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits