AST: Rename debugger pretty printer Type::dumpPrint to print
#77920
Conversation
This name is consistent with other output API and, thus, more discoverable.
AnthonyLatsis
requested review from
hborla,
slavapestov,
xedin and
tshortli
as code owners
| @@ -7633,7 +7633,7 @@ std::string TypeBase::getStringAsComponent(const PrintOptions &PO) const { | |||
| return OS.str(); | |||
| } | |||
|
|
|||
| void TypeBase::dumpPrint() const { | |||
| void TypeBase::print() const { | |||
There was a problem hiding this comment.
This is actually inconsistent. "print" is never used for debug dumping. You can see this by doing a git grep of SWIFT_DEBUG_DUMPER in the code base. Just a small sample:
swift % git grep SWIFT_DEBUG_DUMPER
include/swift/AST/ActorIsolation.h: SWIFT_DEBUG_DUMPER(dump());
include/swift/AST/ActorIsolation.h: SWIFT_DEBUG_DUMPER(dumpForDiagnostics());
include/swift/AST/Attr.h: SWIFT_DEBUG_DUMPER(dump(const Decl *D = nullptr));
include/swift/AST/Attr.h: SWIFT_DEBUG_DUMPER(dump());
include/swift/AST/AvailabilityScope.h: SWIFT_DEBUG_DUMPER(dump(SourceManager &SrcMgr));
include/swift/AST/Decl.h: SWIFT_DEBUG_DUMPER(dump(const char *filename));
include/swift/AST/Decl.h: SWIFT_DEBUG_DUMPER(dumpRef());
include/swift/AST/DeclContext.h: SWIFT_DEBUG_DUMPER(dumpContext());
include/swift/AST/FileUnit.h: SWIFT_DEBUG_DUMPER(dumpDisplayDecls());
include/swift/AST/FileUnit.h: SWIFT_DEBUG_DUMPER(dumpTopLevelDecls());
It is always some variant of "dumpX". print is always used for the non-debug dump variant that can be called programatically.
There was a problem hiding this comment.
Yeah, in that sense the name is inconsistent. But is that an intentional consistency? I personally find print useful for debugging and want to be able to call it on AST nodes from the debugger by the same name (just as I would by print debugging) without it forcing me to provide arguments, only to fail to resolve some LLVM symbols because my LLVM build is RelWithDebInfo. I also don’t want other contributors to have a hard time figuring out how to spell these out in the debugger.
There was a problem hiding this comment.
@beccadax Hey Becca, I would appreciate an opinion on this.
There was a problem hiding this comment.
dump() means "print detailed s-expression notation" while print() is "print this node as Swift source". Renaming dumpPrint() to print() is fine, because it just calls the other overload of print().
We similarly overload dump() for no arguments vs a stream argument as well.
| @@ -7633,7 +7633,7 @@ std::string TypeBase::getStringAsComponent(const PrintOptions &PO) const { | |||
| return OS.str(); | |||
| } | |||
|
|
|||
| void TypeBase::dumpPrint() const { | |||
| void TypeBase::print() const { | |||
There was a problem hiding this comment.
dump() means "print detailed s-expression notation" while print() is "print this node as Swift source". Renaming dumpPrint() to print() is fine, because it just calls the other overload of print().
We similarly overload dump() for no arguments vs a stream argument as well.
|
@swift-ci please smoke test |
This name is consistent with other output API and, thus, more discoverable.