[PATCH] D56639: NFC: Move Type Visit implementation to TextNodeDumper

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 14 05:42:55 PST 2019


aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some minor nits.



================
Comment at: lib/AST/ASTDumper.cpp:434
+    NodeDumper.Visit(T);
     if (!T) {
       return;
----------------
Elide braces.


================
Comment at: lib/AST/TextNodeDumper.cpp:15
 #include "clang/AST/TextNodeDumper.h"
 
+#include "clang/AST/LocInfoType.h"
----------------
Spurious whitespace.


================
Comment at: lib/AST/TextNodeDumper.cpp:142
+  }
+  if (llvm::isa<LocInfoType>(T)) {
+    {
----------------
Drop the `llvm::`


================
Comment at: lib/AST/TextNodeDumper.cpp:163-166
+  if (T->isDependentType())
+    OS << " dependent";
+  else if (T->isInstantiationDependentType())
+    OS << " instantiation_dependent";
----------------
Can you add a newline before the `if` statement and after the `else if` body? Something to set it apart that only one of these two branches is output.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56639/new/

https://reviews.llvm.org/D56639





More information about the cfe-commits mailing list