[PATCH] D55068: NFC: Simplify dumpStmt child handling

Fri Nov 30 06:18:24 PST 2018

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