NFC: Implement OMPClause dump in terms of visitors

[steveire]

Update

[aaron.ballman]

LGTM aside from a nit.

lib/AST/ASTDumper.cpp

1464

const auto *

lib/AST/TextNodeDumper.cpp

276–280

This pattern is coming up quite frequently. I wonder if we should factor this out into something like:

template <typename Ty>
bool dumpNullObject(const Ty *Val, StringRef Label) const {
  if (!Val) {
    ColorScope Color(OS, ShowColors, NullColor);
    OS << "<<<NULL>>> " << Label;  
  }
  return !Val;
}

So that uses become:

if (dumpNullObject(Whatever, "Whatever"))
  return;

Doesn't have to be done as part of this patch, but it seems like it might reduce some redundancy.

[steveire]

lib/AST/TextNodeDumper.cpp

276–280

Something to come back to later I think. It would be more consistent now to have the label on the left side (dumping the various parts of an if() which may be nullptr), and in this OMPClause case, to not have a label on the right side.

Also - at least the 'OS << "<<<NULL>>>"' in the comment visitor is dead unreachable code.

include/clang/AST/TextNodeDumper.h

public:
   void Visit(const Type *T);
 
   void Visit(QualType T);
 
   void Visit(const Decl *D);
 
   void Visit(const CXXCtorInitializer *Init);
 
+  void Visit(const OMPClause *C);
+
   void dumpPointer(const void *Ptr);
   void dumpLocation(SourceLocation Loc);
   void dumpSourceRange(SourceRange R);
   void dumpBareType(QualType T, bool Desugar = true);
   void dumpType(QualType T);
   void dumpBareDeclRef(const Decl *D);
   void dumpName(const NamedDecl *ND);
   void dumpAccessSpecifier(AccessSpecifier AS);

lib/AST/ASTDumper.cpp

public:
 
     // Stmts.
     void VisitDeclStmt(const DeclStmt *Node);
     void VisitAttributedStmt(const AttributedStmt *Node);
     void VisitCXXCatchStmt(const CXXCatchStmt *Node);
     void VisitCapturedStmt(const CapturedStmt *Node);
 
     // OpenMP
+    void Visit(const OMPClause *C);
     void VisitOMPExecutableDirective(const OMPExecutableDirective *Node);
 
     // Exprs
     void VisitInitListExpr(const InitListExpr *ILE);
     void VisitBlockExpr(const BlockExpr *Node);
     void VisitOpaqueValueExpr(const OpaqueValueExpr *Node);
     void VisitGenericSelectionExpr(const GenericSelectionExpr *E);
 

 void ASTDumper::VisitCapturedStmt(const CapturedStmt *Node) {
   dumpDecl(Node->getCapturedDecl());
 }
 
 //===----------------------------------------------------------------------===//
 //  OpenMP dumping methods.
 //===----------------------------------------------------------------------===//
 
+void ASTDumper::Visit(const OMPClause *C) {
+  for (auto *S : C->children())
+    dumpStmt(S);
+}
+
 void ASTDumper::VisitOMPExecutableDirective(
     const OMPExecutableDirective *Node) {
   for (auto *C : Node->clauses()) {
     dumpChild([=] {
-      if (!C) {
-        ColorScope Color(OS, ShowColors, NullColor);
-        OS << "<<<NULL>>> OMPClause";
-        return;
-      }
-      {
-        ColorScope Color(OS, ShowColors, AttrColor);
-        StringRef ClauseName(getOpenMPClauseName(C->getClauseKind()));
-        OS << "OMP" << ClauseName.substr(/*Start=*/0, /*N=*/1).upper()
-           << ClauseName.drop_front() << "Clause";
-      }
-      NodeDumper.dumpPointer(C);
-      NodeDumper.dumpSourceRange(SourceRange(C->getBeginLoc(), C->getEndLoc()));
-      if (C->isImplicit())
-        OS << " <implicit>";
-      for (auto *S : C->children())
-        dumpStmt(S);
+      NodeDumper.Visit(C);
+      Visit(C);
     });
   }
 }
 
 //===----------------------------------------------------------------------===//
 //  Expr dumping methods.
 //===----------------------------------------------------------------------===//
 
 
 void ASTDumper::VisitInitListExpr(const InitListExpr *ILE) {

lib/AST/TextNodeDumper.cpp

if (Init->isAnyMemberInitializer()) {
     dumpType(QualType(Init->getBaseClass(), 0));
   } else if (Init->isDelegatingInitializer()) {
     dumpType(Init->getTypeSourceInfo()->getType());
   } else {
     llvm_unreachable("Unknown initializer type");
   }
 }
 
+void TextNodeDumper::Visit(const OMPClause *C) {
+  if (!C) {
+    ColorScope Color(OS, ShowColors, NullColor);
+    OS << "<<<NULL>>> OMPClause";
+    return;
+  }
+  {
+    ColorScope Color(OS, ShowColors, AttrColor);
+    StringRef ClauseName(getOpenMPClauseName(C->getClauseKind()));
+    OS << "OMP" << ClauseName.substr(/*Start=*/0, /*N=*/1).upper()
+       << ClauseName.drop_front() << "Clause";
+  }
+  dumpPointer(C);
+  dumpSourceRange(SourceRange(C->getBeginLoc(), C->getEndLoc()));
+  if (C->isImplicit())
+    OS << " <implicit>";
+}
+
 void TextNodeDumper::dumpPointer(const void *Ptr) {
   ColorScope Color(OS, ShowColors, AddressColor);
   OS << ' ' << Ptr;
 }
 
 void TextNodeDumper::dumpLocation(SourceLocation Loc) {
   if (!SM)
     return;