steveire marked an inline comment as done. steveire added inline comments.
================ Comment at: lib/AST/ASTDumper.cpp:1987 + ConstStmtVisitor<ASTDumper>::Visit(S); + ---------------- aaron.ballman wrote: > steveire wrote: > > aaron.ballman wrote: > > > Was there something special about calling the Visit methods directly and > > > bailing out? Your code certainly looks reasonable, but I wonder if the > > > output is changed because it goes through the `ConstStmtVisitor` instead > > > of directly dispatching. > > I don't think it could have changed. By my understanding of the > > `StmtVisitor`, this has the same effect. > Yeah, I don't see how it would either, but the original code smells > suspicious to me. How about adding some tests for DeclStmt and > GenericSelectionExpr that demonstrates explicitly this isn't changing > behavior? > > In fact, it seems that we have some bugs in this area (with DeclStmt) > already, so understanding what's changing may point out fixes. > https://godbolt.org/z/tDJ-0H A visual inspection and reading of the visitor should show that this patch is NFC. I'd welcome a test if you can make one. The test should be in the repo before this commit anyway. > In fact, it seems that we have some bugs in this area (with DeclStmt) > already, so understanding what's changing may point out fixes. > https://godbolt.org/z/tDJ-0H That is not related to this refactoring. Also, clang-query has different output: http://ec2-52-14-16-249.us-east-2.compute.amazonaws.com:10240/z/W_kZFl Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55068/new/ https://reviews.llvm.org/D55068 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
