This is an archive of the discontinued LLVM Phabricator instance.

Extract TextNodeDumper class
ClosedPublic

Authored by steveire on Dec 3 2018, 1:45 AM.

Details

Summary

Start by moving some utilities to it. It will eventually house dumping
of individual nodes (after indentation etc has already been accounted
for).

Diff Detail

Repository
rC Clang

Event Timeline

steveire created this revision.Dec 3 2018, 1:45 AM

Basically looking good, modulo namespace questions from D55188 and a few other organizational questions.

include/clang/AST/ASTTextNodeDumper.h
1 ↗(On Diff #176322)

Perhaps the file should be named TextNodeDumper.h to match the class name?

44 ↗(On Diff #176322)

Should these implementations live in the header file? I feel like they belong in a .cpp file instead.

steveire updated this revision to Diff 176478.Dec 3 2018, 2:20 PM

Move implementation to cpp file

aaron.ballman accepted this revision.Dec 4 2018, 2:13 PM

LGTM!

lib/AST/ASTDumper.cpp
90

Another nice cleanup for later would be to replace these call sites with the NodeDumper.foo() version. Same for dumpBareDeclRef() below.

This revision is now accepted and ready to land.Dec 4 2018, 2:13 PM
steveire marked an inline comment as done.Dec 5 2018, 12:23 PM
steveire added inline comments.
lib/AST/ASTDumper.cpp
90

These methods are called from AttrDump.inc. They'll be removed when the commit to port the Attr dumping goes in.

This revision was automatically updated to reflect the committed changes.